Repository navigation
Conversation
JulienVig
left a comment
JulienVig
left a comment
There was a problem hiding this comment.
Good idea to reduce unecessary dependencies! I think we can push this further for the Docker smoke test. However it looks like some dependencies are missing for it in ci-paths.yml .
Looking at the past CI runs it looks like the speedup is pretty unsignificant compared the timing of test/server. It doesn't cost anything to parallelize more but maybe we should focus our efforts on splitting test/server in several parallel jobs?
| steps: | ||
| - uses: actions/checkout@v7 | ||
| - uses: docker/setup-buildx-action@v4 | ||
| - run: docker build . |
There was a problem hiding this comment.
Is this duplicate build step needed?
| server_docker: | ||
| - "Dockerfile" | ||
| - ".dockerignore" | ||
| - "server/**" |
There was a problem hiding this comment.
These dependencies look too narrow no? the Dockerfile depends on discojs, discojs-node and isomorphic-webrtc, you could add *discojs-node as dependency.
| discojs_node: ${{ needs.affected.outputs.discojs_node == 'true' }} | ||
| discojs_web: ${{ needs.affected.outputs.discojs_web == 'true' }} | ||
| server: ${{ needs.affected.outputs.server == 'true' }} | ||
| server_docker: ${{ needs.affected.outputs.server_docker == 'true' }} |
There was a problem hiding this comment.
putting the docker smoke test here means that we wait for static analysis, all builds, dataset downloads before running the test. What about putting it in a separate test that only needs affected?
What & why
Currently in
verify.yml:download-datasetsrestoresdatasets/fromactions/cache@v5, and then re-zips and re-uploads the entire folder toactions/upload-artifact@v7on every single CI run so test jobs can pull it withactions/download-artifact.buildwaits ondownload-datasets(needs: [download-datasets, static-analysis]) even though_build.ymlnever downloads or usesdatasets/.build-server-docker(~2m11s) is bundled inside_build.ymlalongsidebuild-topological(~1m06s), forcing all 14 test jobs in_test.ymlto sit idle for an extra ~1 minute waiting for the Docker smoke test even thoughall-buildsis already ready and no test job uses the Docker image.Changes
.github/actions/download-datasets/action.yml&pull-dataset-artifacts/action.yml):lookup-only: trueonactions/cache@v5indownload-datasetsso cache hits complete in seconds without downloading or re-uploading a duplicate workflow artifact.datasets/directly fromactions/cache/restore@v5(withfail-on-cache-miss: true) inpull-dataset-artifacts..github/workflows/verify.yml&_build.yml):download-datasetsfrombuild.needstotest.needs.build-server-dockerout of_build.ymlintoverify.yml(needs: [static-analysis]) so it runs in parallel withbuildandtestinstead of blockingteststartup.