Improve error message when Visual Studio can't be found#1691
Improve error message when Visual Studio can't be found#1691danyeaw merged 3 commits intowingtk:mainfrom
Conversation
mitchhentges
force-pushed
the
improve-vs-find-error
branch
2 times, most recently
from
a71f0db to
a1693d9
Compare
danyeaw
left a comment
danyeaw
left a comment
There was a problem hiding this comment.
Hey @mitchhentges - thanks for the contribution! I know our current test coverage isn't great, but would you mind adding a couple of tests to check that the updated logic is working correctly?
|
Good idea :) I've been thinking more about this, and I think I should bundle this "list of each ineligible VS installation" with the above "list of VS installations that will be checked". I'll do that wider change, then add some tests 👍 |
mitchhentges
force-pushed
the
improve-vs-find-error
branch
2 times, most recently
from
d04ee1b to
71a0150
Compare
|
I've still got tests to add, but here's how it looks when I've combined "failure reason" with "list of detected installations": Failure case: One failure but one usable installation case: 
I'll follow up with tests "soon" 😉 |
This addresses the confusion of `gvsbuild` showing that it _found_ Visual Studio installations, but then logging right after that it couldn't, in fact, find it. This adds logs for each VS Installation that it considers "inapplicable", such as because: * The version doesn't match `vs_ver` * The `vcvars.bat` is missing ----- The `exit_missing` parameter was removed because it was _always_ set to `False`.
It's a bit tricky to test, since this function is both private and also used by a pretty heavy constructor. I've opted to skip the constructor with `__new__`, and then do the `_$class__$fn()` technique to call it directly. This way, I can ensure that the test is tight, and that no unexpected code runs (e.g. the tests shouldn't call any real scripts or touch the file system in any funky way).
mitchhentges
force-pushed
the
improve-vs-find-error
branch
from
71a0150 to
aec49f2
Compare
|
Hey, let me know what you think! I've added some tests for the VS Installation checking code, and IMHO my logging changes are simple enough (and adding tests for 'em would be hard enough) that hopefully (🤞 ) they're good as-is! Thanks for your patience here 🫡 |
danyeaw
left a comment
danyeaw
left a comment
There was a problem hiding this comment.
Hey @mitchhentges, these changes look great and thanks for adding tests. Just a couple of minor cleanups I saw.
mitchhentges
force-pushed
the
improve-vs-find-error
branch
from
89e416b to
1d27880
Compare
2 participants
This addresses the confusion of
gvsbuildshowing that it found Visual Studio installations, but then logging right after that it couldn't, in fact, find it.This tweaks the list of detected VS installations to also include logs when an installation is considered "inapplicable", such as because:
vs_vervcvars.batis missing