-
Notifications
You must be signed in to change notification settings - Fork 31
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
Consolidate path tests #258
Conversation
This reverts commit b97ec5c.
So I think this pull request can be merged (after #257) safely without reviewing details, or I'd like to merge this by myself in a few days. |
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd
…e-path-tests # Conflicts: # appveyor.yml
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd
…e-path-tests # Conflicts: # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…e-path-tests # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
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 looks good to me and it is in line with the pull requests previously merged. It is useful to consolidate these tests in order to speed up the execution of the test suite on Travis and AppVeyor (we now have many more tests to run and the test suite has increased the total execution time, especially on AppVeyor).
I reviewed this pull request by inspecting the code, making sure that both tests are covered in the path, and running the test suite locally on Linux and Windows.
I'm merging this into master
.
xspec.bats
(a test script for *nix) andxspec-bat.cmd
(a test script for Windows) have 2 tests for path related issues (#84 and #119).Though they need to check the same things, each of them runs XSpec individually (with a different path).
This pull request consolidates them and runs XSpec just once (with a combined path).
No code change in XSpec core.
This pull request derives from #257. So needs to be handled after that.