fix: address flaky integration tests #7890
Conversation
jaredm563
force-pushed
the
jared/address-flaky-tests
branch
from
d5e2c89 to
aff59d3
Compare
jaredm563
changed the title
| // If a build is already in progress, mark that we need another build | ||
| // and return the current build's promise. The running build will | ||
| // trigger a rebuild when it completes if buildPending is true. | ||
| if (this.buildPromise) { |
There was a problem hiding this comment.
can we add tests here for this method which will ensure this tests fine?
this.build() // resolved firstly
this.build() // resolved together with the next
this.build() // resolved together with the previous
There was a problem hiding this comment.
^ and the test should verify that subsequent calls trigger an actual new build!
| // If a build is already in progress, mark that we need another build | ||
| // and return the current build's promise. The running build will | ||
| // trigger a rebuild when it completes if buildPending is true. |
There was a problem hiding this comment.
👀 Instead of reimplementing this, we could reuse the existing battle-tested helper we already use for this same purpose for (non-Edge) Functions! Check it out:
.There was a problem hiding this comment.
Good idea thank you!
There was a problem hiding this comment.
So I looked into implementing that and unfortunately if build A fails while build B is pending, the caller of build A gets the error but build B runs without anyone waiting for it. I'll add a comment explaining why there is a custom implementation
🎉 Thanks for submitting a pull request! 🎉
Summary
Fix 1:
Multiple file watcher events (onAdd, onChange) could trigger concurrent build() calls, causing race conditions where the bundler accessed functionConfigs[name] before it was populated .
now only one build runs at a time, and when the current build completes, it checks buildPending and runs another build if needed
Fix 2:
tests/integration/utils/dev-server.ts waited up to 90s with no useful error message, now on timeout it should shows the actual server output so far for debugging
Fix 3:
In tests/integration/commands/init/init.test.ts answerWithValue(CONFIRM) sent ['\n', '\n'] (two newlines), which was causing the second newline to answer the wrong prompt.
Now it should use a single CONFIRM for list/confirm selections, so that prompts are answered correctly and the netlify.toml gets created
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)