-
Notifications
You must be signed in to change notification settings - Fork 985
More unit testifications #2416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More unit testifications #2416
Conversation
One integration test kept as a smoke test for insurance.
e868c0a to
07d031f
Compare
kinnison
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks consistent, modulo the -gnu failure case we need to resolve, but what's that comment all about?
2c42bf3 to
86b0459
Compare
Just reporting 'stable' is less clear than we can be, and inconsistent, since a reinstall when a default toolchain has been selected will already report the toolchain desc (via toolchain.name()). Also convert install_sets_up_stable into unit test. This involves making a bunch of test support code available to unit tests, and breaking maybe_install_rust into a calculating part that can be tested, as well as a simple execution part which acts on the calculated result, which we can test separately in functional tests / tests of that underlying layer / integration tests. this_host_triple in addition to being moved has been made somewhat more faithful to from_host_or_build by always returning msvc for windows hosts. Additionally, since it doesn't need to override the BUILD_TRIPLE, that was removed from it.
kinnison
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all seems to make sense.
|
You are right that it took quite a kicking from some heavy boots, but I think this is good. |
See the individual commits please