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(framework:skip) Add caching to speed up E2E CI tests #3480

Merged
merged 25 commits into from
May 23, 2024

Conversation

chongshenng
Copy link
Contributor

@chongshenng chongshenng commented May 21, 2024

Issue

Description

This PR improves the speed of the CI for E2E tests.

Related issues/PRs

This PR is inspired by #1975 and this blogpost by Evan Pete Walsh. The CI should be triggered couple of times after that has happened to compare the impact of the caching.

Explanation

The CI cache works as follows:

main branch

Whenever a new commit or PR is made to the main branch the Cache Python location job in .github/workflows/e2e.yml will run and cache the following directory:

  • ${{ env.pythonLocation }} (Note that caching only ~/.cache/pip will cache the wheels, but not the installation itself)

The cache will be overwritten if there is a cache miss. It is retrieved by jobs in pull requests by the following key:

pip-${{ runner.os }}-${{ matrix.directory }}-${{ env.pythonLocation }}-${{ hashFiles('**/pyproject.toml') }}

We ensure that the job retrieving the cache must meet the following requirements:

  1. Same OS
  2. Same matrix directory (important because matrix is used in the E2E test)
  3. Same Python location (which looks something like /opt/hostedtoolcache/Python/3.8.18/x64)
  4. No changes to pyproject.toml in the project folders.

One significant update in this PR is that all pyproject.tomls in the E2E folder have been migrated to pure pip installation with hatchling backend without relying on poetry and setuptools.

In this PR, we only rely on actions@cache for caching the Python location instead of actions@setup-python because the latter does not yet work with matrix configurations due to cache key conflicts. This is explained by the setup-python maintainers in this thread.

Pull Requests

Pull requests will try to use the cache by doing a lookup using the previously described key and pull the cache is available.

Preliminary results

E2E timings can reduce down to ~4 min 50+ secs by caching the Python location, compared to ~5 min 20+ secs if we cache only ~/.cache/pip, and ~5 min 40+ secs without caching (see this workflow run).

Checklist

  • Implement proposed change
  • [ ] Write tests
  • Update documentation
  • Update the changelog entry below
  • Make CI checks pass
  • Ping maintainers on Slack (channel #contributions)

Changelog entry

Any other comments?

N/A

@chongshenng chongshenng changed the title ci(framework:skip) Speed up CI ci(framework:skip) Add caching to speed up CI May 21, 2024
@chongshenng chongshenng changed the title ci(framework:skip) Add caching to speed up CI ci(framework:skip) Add caching to speed up E2E CI tests May 21, 2024
@chongshenng chongshenng marked this pull request as ready for review May 22, 2024 09:46
@chongshenng chongshenng self-assigned this May 22, 2024
- name: Install dependencies
run: python -m poetry install
run: python -m pip install .
Copy link
Member

@Robert-Steiner Robert-Steiner May 22, 2024

Choose a reason for hiding this comment

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

We could add -U flag to always upgrade packages to the newest available version.
Example:
If we have the requirement pytest>=7.3,<8 and the cache already contains the version 7.3, pip install does not install the latest version 7.4.1 because pip prefers the installed version unless --upgrade or -U is specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, let me investigate.

@chongshenng chongshenng marked this pull request as draft May 22, 2024 12:17
@chongshenng chongshenng marked this pull request as ready for review May 22, 2024 12:54
Copy link
Member

@tanertopal tanertopal left a comment

Choose a reason for hiding this comment

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

LGTM

@tanertopal tanertopal enabled auto-merge (squash) May 23, 2024 16:12
@tanertopal tanertopal merged commit a43aa0d into main May 23, 2024
29 checks passed
@tanertopal tanertopal deleted the improve-ci-speed branch May 23, 2024 16:17
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.

4 participants