Fix Flaky TestScripts#22838
Conversation
…testScriptsStable
product-security-plaid-production
Bot
requested review from
jmank88 and
patrickhuie19
|
👋 kalverra, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
|
| // If we're not recursing, skip all other directories except the root. | ||
| if !bool(d.recurse) && !isRootDir { | ||
| return nil | ||
| if !bool(d.recurse) && filepath.Clean(path) != root { |
There was a problem hiding this comment.
Is it safe to use this less strict filepath.Clean comparison instead of os.SameFile?
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func writeTreeFile(t *testing.T, path string) { |
There was a problem hiding this comment.
Would it make sense to use embedded testdata/ dir files rather than creating them along the way?
|
|
| t.Parallel() | ||
|
|
||
| require.NoError(t, os.Setenv("TMPDIR", "/tmp")) // osx default is too long for go-plugin sockets | ||
| require.NoError(t, os.Setenv("GOTMPDIR", "/tmp")) // keep workspaces in /tmp |
There was a problem hiding this comment.
Should we use the t.TempDir()?
| ContinueOnError: true, | ||
| Files: filesToRun, | ||
| Setup: commonEnv(t), | ||
| ContinueOnError: false, |
There was a problem hiding this comment.
Why not? If there are multiple failing tests, we would rather know about them all at once.
2 participants
TestScriptsis one of the most flaky areas in /chainlink. I ran the fix-flaky-tests skill on it withopus 4.8. It cost ~$2.00 and took ~30 minutes (excluding time to run the tests over and over to verify fixes).Changes
StartUpHealthReportnow returns503errors for non/healthpaths instead of the old404error. This is more in line with what most clients expect for this sort of situation.curl --retryis used in TestScripts to keep calling these endpoints until they're ready, but it will not retry if it gets a 404.tools/txtar/visitor.goand updated it with some more modern Go patterns, should help performance a teeny-tiny bit.Results