chore(repo): Test task cache, instance cleanup - #4385
Conversation
|
brkalow
changed the title
panteliselef
left a comment
panteliselef
left a comment
There was a problem hiding this comment.
What's the reason for dropping apiUrl. Unused property ?
| timeout-minutes: ${{ vars.TIMEOUT_MINUTES_LONG && fromJSON(vars.TIMEOUT_MINUTES_LONG) || 15 }} | ||
|
|
||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
Given the flakiness of our integration tests, it feels valuable to continue running tests even if others fail. Many times tests are minutes into running when cancelled, which I don't think has the cost savings we expect in practice.
| if (!secretKey) { | ||
| return null; | ||
| } | ||
| return { secretKey, apiUrl: json.private['CLERK_API_URL'] }; |
There was a problem hiding this comment.
apiUrl seemed unused as far as I can tell, or at least not changed from the default.
| import { appConfigs } from '../presets/'; | ||
|
|
||
| setup('cleanup instances ', async () => { | ||
| const entries = Object.values(appConfigs.envs) |
There was a problem hiding this comment.
appConfig.envs didn't contain all the keys in use, so not all instances were getting cleaned up as far as I could tell.
| }, | ||
| "//#test:integration:ap-flows": { | ||
| "dependsOn": ["^@clerk/clerk-js#build", "^@clerk/backend#build", "^@clerk/nextjs#build"], | ||
| "dependsOn": ["@clerk/clerk-js#build", "@clerk/backend#build", "@clerk/nextjs#build"], |
There was a problem hiding this comment.
All of the adjustments here remove the ^ for each dependency. The ^ says to wait on the package's dependencies, which for a root task like this effectively seems like a no-op.
I've validated that this properly caches the integration test run only if the dependent build steps remain unchanged, which is the desired behavior.
4 participants
Description
Fixes caching for integration test tasks, all task dependencies incorrectly contained
^, which means the task should depend on its package's dependencies running the tasks specified with^. This was effectively a no-op when used for a root task with package-specific tasks in its dependencies. This should fix integration test caching 🤞Also fixes an issue with the integration test cleanup script where not all instances were being cleaned up.
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change