You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
This could have consequences or limit some things we could try to do with tests. Locally the local server tests went from 3m to ~ 1m10s. Trying it out for now to see how much impact they have on PR build runtime.
Reviewer Guidance
This brought the e2e local server tests from 3m30s to 1m12s, and the e2e tinylicious tests from 15m27s to an amazing 1m16s... but the number of tests doesn't match? And more surprisingly, this change ran more tests than current PRs normally do.
I think the discrepancy for those tests is explained by this logic to skip them, which probably is applying correctly today, but somehow gets broken by --parallel. Maybe some environment variables are not getting passed correctly to the worker processes.
Bit of an update on this... the reason the number of tinylicious tests didn't match when running in parallel is that the way we're specifying the FF test options (like the driver) doesn't work with mocha's --parallel flag. We pass them as CLI flags when invoking mocha, but when mocha runs in parallel mode it doesn't pass all its CLI flags to the worker processes that actually run the tests, so in this case the worker processes were running tests with default settings, which meant local driver. I made adjustments to work around this, and the the e2e tinylicious tests started taking pretty much the same amount of time in serial and parallel modes, but with many more failures in the parallel case, so it would seem that for tinylicious in particular it doesn't make sense to parallelize the tests (for now). I'm investigating if there are improvements when running against our internal r11s deployment.
area: testsTests to add, test infrastructure improvements, etcbase: mainPRs targeted against main branch
2 participants
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.
github-actions
bot
added
area: tests
alexvy86
force-pushed
the
parallel-tests
branch
from
Description
This could have consequences or limit some things we could try to do with tests. Locally the local server tests went from 3m to ~ 1m10s. Trying it out for now to see how much impact they have on PR build runtime.
Reviewer Guidance
This brought the e2e local server tests from 3m30s to 1m12s, and the e2e tinylicious tests from 15m27s to an amazing 1m16s... but the number of tests doesn't match? And more surprisingly, this change ran more tests than current PRs normally do.
For tinylicious the run without this change says 3874 passing, 1232 pending, whereas the run with this change says 4697 passing, 409 pending. As an example, the "Summarizer with local changes" tests ran for this PR, but are being skipped today.
I think the discrepancy for those tests is explained by this logic to skip them, which probably is applying correctly today, but somehow gets broken by
--parallel. Maybe some environment variables are not getting passed correctly to the worker processes.