-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Running CircleCI render tests on Windows, Mac and Firefox #12268
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
SnailBones
added
testing 💯
skip changelog
Used for PRs that do not need a changelog entry
labels
Sep 29, 2022
SnailBones
changed the title
Running CircleCi render tests on Windows and Mac virtual machines
Running CircleCI render tests on Windows and Mac virtual machines
Sep 30, 2022
karimnaaji
reviewed
Oct 20, 2022
test/integration/render-tests/debug/terrain/collision-pitch-with-map/style.json
Outdated
Show resolved
Hide resolved
SnailBones
commented
Oct 21, 2022
@@ -2,8 +2,7 @@ | |||
"version": 8, | |||
"metadata": { | |||
"test": { | |||
"height": 256, | |||
"allowed": 0.00025 | |||
"height": 256 |
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 test was incorrectly reverted in #10562 and was passing due to a high "allowed." By removing the allowed and correcting the test we should now correctly catch a regression to order.
…images that were previously offscreen
This reverts commit 0cb12f9.
3 tasks
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.
Closes https://github.com/mapbox/gl-js-team/issues/480
Adds three new runs in CircleCI:
Windows and Mac tests are running on virtual machines.
Failing tests on Windows and Mac are due to bugs including #7331. Using
--use-gl-desktop
lets these tests pass on Mac but also causes a different set of failing tests due to missing globes, inconsistent stars and fill extrusion patterns (as was the reason for disabling this locally in #12090).The same tests skipped on Windows and Mac formerly failed on Linux without the
--use-gl-desktop
flag (added in #10389) as seen in this run without the flag.I've taken the following approach to updating "allowed" properties:
Previous runs tended to finish in 5-6 minutes. All of the new runs added take longer:
To address this, I've implemented parallel runs on Mac and Windows. This has required some restructuring of how reading testfiles work. If a
tests-to-run.txt
file is present in the root directory, the test runner will read tests from that file instead of running all tests that match a glob.There's likely room to optimize the CI config through caching. but 90% of the time spent on Windows is running tests. A more effective way to speed up CI outputs would be splitting it into two (or more) runs, with each running half the tests.
Issues tracking new tests in ignores:
Launch Checklist