-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Refactor common test suite #377
Merged
RST-J
merged 4 commits into
voxpupuli:master
from
iainbeeston:refactor-common-test-suite
Mar 7, 2017
Merged
Refactor common test suite #377
RST-J
merged 4 commits into
voxpupuli:master
from
iainbeeston:refactor-common-test-suite
Mar 7, 2017
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
2cde92e
to
d7151d0
Compare
With draft6 coming we have a lot more ignored tests (for now, until we can fix the code to support new features). Rather than just add these all onto the current hash of ignored tests, I've refactored them out to a yaml file. I've also changed how we ignore tests that we don't support. Previously any test that we didn't support simply didn't have a test created for it, in the test suite. Instead I have changed it so that only optional tests are handled in this way. Non-optional (ie core) tests that we don't support are skipped instead, so they are visible to anyone running the tests, and so that we don't forget that we should implement those at a future date.
When the default rake task is run (which is the standard way to run the tests) we always update the common test suite before running the tests. Unfortunately, if the computer running the tests has no internet access, this update will fail and then the tests never run. I've changed this so that if the update to the common test suite fails, we display an error then carry on with the rest of the test suite.
For quite while we've made the test rake task update the common test suite every time it's run, to stop the test suite from becoming out-of-date. However, the downside to this is that many pull requests end up failing due to new tests in the common test suite, rather than changes in the pull request itself. I think it'd be best to manually update the common test suite in future. It should make the test suite more reliable.
d7151d0
to
06d1d94
Compare
@RST-J Do you have time to code review this pull request? It should fix the failing tests in the common test suite and hopefully stop them failing unexpectedly in future |
@RST-J thanks! |
iainbeeston
added a commit
to iainbeeston/json-schema
that referenced
this pull request
Jul 5, 2017
In voxpupuli#377 I made it possible to run the tests without having internet access. But it seems that in the process I also disabled the code that downloads the common test suite when you first check out the codebase. That's bad, but what makes it worse is that travis hasn't been running the common test suite because of it. This changes the behaviour so that the tests download the common test suite (but does not update it) before running. If you don't have internet access it continues without it. To update the common test suite to the latest version, you now need to run `UPDATE_TEST_SUITE=true rake download_common_tests`.
iainbeeston
added a commit
to iainbeeston/json-schema
that referenced
this pull request
Jul 5, 2017
In voxpupuli#377 I made it possible to run the tests without having internet access. But it seems that in the process I also disabled the code that downloads the common test suite when you first check out the codebase. This changes the behaviour so that if you run the test suite and you have internet access the common test suite is automatically downloaded. If you don't have internet access it continues without it. To update the common test suite to the latest version, you now need to run the tests with the `UPDATE_TEST_SUITE` env var set.
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 pull request contains a number of improvements to how we use the common test suite:
Please see the commit messages for more details of each change