Minor changes and fixes for new test suite #583
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Install python 2 and avoid using virtualenv, pip on osx works fine without it. Also remove unnecessary path prefix and add newline at the end.
When parsing test options, instead of directly looking for '-Xhelper' in arguments and splitting the next arguments on commas add .busted config to set test/test_environment as busted helper. Then busted will do the splitting on its own and set global arg to split options.
Return false if installation failed. Also, remove optional arguments.
Use it instead of os.rename(path, path).
|
Wow, thanks Peter! That's a thorough review! I'm glad you like the new test suite, @robooo has been putting a lot of work on it (and it's awesome that we already got it to a mergeable state and he's still basically halfway through his GSoC!) If both you and @robooo are fine with all these changes, I think they are good to merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is just a heap of commits adjusting the new test suite a little. Not sure if this can be merged completely, will probably refine and cherry-pick commits one by one.
So far I like the new test suite a lot. Implementing it using busted allows us to later improve it further:
assert.is_trueandassert.is_falseassertions that are used when checking command success and failure (it just prints:expected true, got false). If we'll go this way we'll need custom assertions that execute command and print it and its output on unexpected result. A collections of such assertions would be valuable for testing other command-line programs, might want to make that a library.