Skip to content
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

Add CLI tests #1604

Merged
merged 3 commits into from
Sep 26, 2021
Merged

Add CLI tests #1604

merged 3 commits into from
Sep 26, 2021

Conversation

notriddle
Copy link
Contributor

Part of #1568

@notriddle notriddle force-pushed the notriddle/test-cli branch 6 times, most recently from 17c7088 to 5a07fe6 Compare July 14, 2021 20:11
@notriddle
Copy link
Contributor Author

Finally, I managed to make a version that actually works correctly under Windows.

@joshrotenberg
Copy link
Contributor

This is great, thanks for kicking it off!

Obviously there is a lot to test, and doing it in small chunks makes sense. One suggestion I have would be to make a test/cli top level module, and then separate each test file to match the cli files, so like:

tests/cli/mod.rs  - any shared test stuff can go here
tests/cli/build.rs - all tests for the build command
...

@notriddle
Copy link
Contributor Author

@joshrotenberg

I can do that. I don't really like the result, because you're required to have a top-level .rs file for the test harness to actually run the tests. But it is doable.

@notriddle notriddle force-pushed the notriddle/test-cli branch from 92a3476 to 7afded5 Compare July 29, 2021 21:51
@ehuss ehuss force-pushed the notriddle/test-cli branch from 7afded5 to 9bede85 Compare September 26, 2021 18:29
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I went ahead and rebased to resolve the conflict. I'm slightly concerned about adding so many dependencies, but I don't think there is an easy way around that.

@joshrotenberg I'm not sure if you had any more feedback on this, but I'm going to go ahead and merge. If you had anything else to add, please let me know.

@ehuss ehuss merged commit a306da3 into rust-lang:master Sep 26, 2021
@notriddle notriddle deleted the notriddle/test-cli branch September 26, 2021 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants