Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: faster ci #3235

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

ci: faster ci #3235

wants to merge 22 commits into from

Conversation

bolinocroustibat
Copy link
Contributor

@bolinocroustibat bolinocroustibat commented Dec 20, 2024

  • Parallelise the execution of the tests in python job using 2 CircleCI executors, similar to what have been done in ci: parellelize tests in CI datagouv/hydra#238, except than here, the tests that are executed in parallel must be manually specified in the CI file, since some tests can't be parallelised. Therefore, the CI code is more complicated.
    Building CI workflow is now around 1m40s faster on average compared to before parallelisation (which is ~30% faster , thanks to ~45% faster specifically on the python-tests step)

Before:
Screenshot 2024-12-23 at 23 39 56

After:
Screenshot 2024-12-23 at 23 39 12

  • Modify inv tests task so we can pass specific tests, to be used by parallelization
  • Add fallback cache in CI

FYI, this other strategies to improve CI speed have also been tried and abandoned:

  • Parallelise the execution of the 3 tasks in assets job using 3 CircleCI executors. -> While this was working well, this didn't result in any time gain as it was already probably executed in parallel
  • Cache npm dependencies more aggressively -> didn't result in any significant improvements
  • Use npm ci instead of npm install. Advantages:
    • Faster installation (can be 2-3x faster) because it skips certain user-oriented features
    • Deterministic installations - strictly uses package-lock.json
    • Removes node_modules before installation to ensure clean slate
    • Won't write to package.json or package-lock.json
      Disadvantage: many issues related to the command not being recognized in later steps

@bolinocroustibat bolinocroustibat force-pushed the faster-ci branch 3 times, most recently from 8077755 to 74972a2 Compare December 21, 2024 06:39
@bolinocroustibat bolinocroustibat force-pushed the faster-ci branch 3 times, most recently from 7edd80a to 4e7cab2 Compare December 21, 2024 09:32
@bolinocroustibat bolinocroustibat self-assigned this Dec 21, 2024
@bolinocroustibat bolinocroustibat marked this pull request as draft December 21, 2024 12:02
…still being executed in first executor as well
@bolinocroustibat bolinocroustibat force-pushed the faster-ci branch 4 times, most recently from 6e7b77b to 25575fa Compare December 23, 2024 14:18
@bolinocroustibat bolinocroustibat marked this pull request as ready for review December 23, 2024 14:49
Copy link
Contributor

@maudetes maudetes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear about the way it works, but don't we now have 1,4K tests instead of 2K?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants