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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH Actions: re-work the integration tests #221

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jan 20, 2022

Description

GH Actions: re-work the integration tests

This removes three keys from the integration test matrix in favour of adding steps for each of those options to the job.

  • The GitHub workspace is cleaned up between each run.
  • The Composer cache is removed between each run.

This means that, in effect, the tests are being run in the same way as previously (clean install), but sequentially in one job instead of in parallel in different builds.

This slims down the number of builds per run from 197 to 29.

GH Actions: run integration tests for additional situations

This adds the tests/fixtures/no-lock-file and tests/fixtures/out-of-sync-lock working directories to the matrix for the integration tests to test these situations more thoroughly on each commit as well.

Fixes #219

(Number of builds per run are now 53)

GH Actions: add extra job for unclean install test

Add a job which tests "unclean" installs, i.e. running the action when there is already a vendor directory and a Composer downloads directory in place.

(Number of builds per run are now 57)

馃啎 Composer: require-dev the package used for testing

.. in the root composer.json.

馃啎 Tests: update version constraints in test fixtures

... to allow for installing a wider range of versions of the ehime/hello-world package.

Includes locking the version for the with-lock-file test to 1.0.3 so there are higher and lower versions to switch to during the tests and switching the "unclean" test to use that fixture as a base.

Motivation and context

See the discussion in #219

How has this been tested?

By running the action and doing a visual check that the output of various steps match the expected output.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

This removes three keys from the integration test matrix in favour of adding steps for each of those options to the job.

* The GitHub workspace is cleaned up between each run.
* The Composer cache is removed between each run.

This means that, in effect, the tests are being run in the same way as previously (clean install), but sequentially in one job instead of in parallel in different builds.

This slims down the number of builds per run from 197 to 29.
This adds the `tests/fixtures/no-lock-file` and `tests/fixtures/out-of-sync-lock` working directories to the matrix for the integration tests to test these situations more thoroughly on each commit as well.

Fixes 219
Add a job which tests "unclean" installs, i.e. running the action when there is already a `vendor` directory and a Composer `downloads` directory in place.
@jrfnl jrfnl requested a review from ramsey as a code owner January 20, 2022 18:42
@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #221 (a4633e0) into v2 (f680dac) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##               v2     #221   +/-   ##
=======================================
  Coverage   98.29%   98.29%           
=======================================
  Files           5        5           
  Lines         117      117           
=======================================
  Hits          115      115           
  Misses          2        2           

@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 20, 2022

Note: the new "unclean" test run isn't very effective at the moment as the ehime/hello-world dependency is fixed at 1.0.5. (same applies to the lowest/highest runs for the integration tests)

@ramsey Would you be open to changing the version constraint, either for the root project or for the locked test, to be a more open version constraint like ^1.0.0 with the locked version being at 1.0.3 ? That way lowest/highest would actually have something to do, making the tests more effective.

On a loosely related note: should the require for the root project for the ehime/hello-world package be changed to require-dev ?

@ramsey
Copy link
Owner

ramsey commented Jan 21, 2022

Would you be open to changing the version constraint, either for the root project or for the locked test, to be a more open version constraint like ^1.0.0 with the locked version being at 1.0.3 ? That way lowest/highest would actually have something to do, making the tests more effective.

This sounds good. I don't recall why I fixed it at 1.0.5.

On a loosely related note: should the require for the root project for the ehime/hello-world package be changed to require-dev ?

Yep. It can be moved to require-dev.

Thanks so much for doing this work! 馃帀

@jrfnl jrfnl force-pushed the feature/219-ghactions-combine-integration-tests branch from 0e35e1d to 5477676 Compare January 21, 2022 03:35
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 21, 2022

I've updated the PR (including the description) with the additional changes.

Happy to help as this is such a useful action ;-)

... to allow for installing a wider range of versions of the `ehime/hello-world` package.

Includes locking the version for the `with-lock-file` test to `1.0.3` so there are higher and lower versions to switch to during the tests and switching the "unclean" test to use that fixture as a base.
@jrfnl jrfnl force-pushed the feature/219-ghactions-combine-integration-tests branch from 5477676 to a4633e0 Compare January 21, 2022 03:38
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 21, 2022

(Sorry for the push noise, there was a weird hickup in the expect tests (in GHA, not related to these changes), so I pushed a non-change to force the tests to re-run)

@ramsey ramsey merged commit 6085843 into ramsey:v2 Jan 24, 2022
@ramsey
Copy link
Owner

ramsey commented Jan 24, 2022

Thank you, Juliette!

@jrfnl jrfnl deleted the feature/219-ghactions-combine-integration-tests branch January 25, 2022 00:09
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 26, 2022

You're very welcome Ben!

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.

CI run job only tests with lock files
2 participants