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

Create GitHub Actions workflow for running tests #944

Merged
merged 8 commits into from Jan 18, 2022
Merged

Create GitHub Actions workflow for running tests #944

merged 8 commits into from Jan 18, 2022

Conversation

samdoran
Copy link
Contributor

@samdoran samdoran commented Dec 17, 2021

Run sanity, unit, and integration tests in GitHub actions. We should be able to disable some Zuul jobs after merging this.

Also upload code coverage reports.

We may want to look at a matrix of Python version X Ansible version. Currently it only tests one Ansible version per Python version.

@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Dec 17, 2021
@samdoran samdoran marked this pull request as ready for review December 17, 2021 18:56
@samdoran samdoran requested a review from a team as a code owner December 17, 2021 18:56
@AlanCoding
Copy link
Member

It looks like this is running the docker and podman tests? This is fantastic. The whole thing ran in under 5 minutes too.

@samdoran
Copy link
Contributor Author

Yup, just running the tests in a VM hosted by GitHub Actions. Ideally we would run those tests in our test container, but I haven't worked how to run podman inside of a Docker container where we don't control the command used to invoke the container.

@samdoran
Copy link
Contributor Author

Adding the image build steps brings overall execution up to a little over nine minutes. Not too bad, but building that image every time is costly.

@Shrews Shrews removed the needs_triage New item that needs to be triaged label Dec 21, 2021
@Shrews
Copy link
Contributor

Shrews commented Dec 21, 2021

Note for @shanemcd and myself to review this after the holidays to make sure we would be covered on both GHA side and Zuul side (global job references?).

@Shrews
Copy link
Contributor

Shrews commented Jan 5, 2022

Note for @shanemcd and myself to review this after the holidays to make sure we would be covered on both GHA side and Zuul side (global job references?).

FYI, there are references to some of the runner jobs in the ansible/ansible project at https://github.com/ansible/project-config/blob/2bd212f0b14ae32bf687865f95cb7c695808ed4f/zuul.d/projects.yaml#L853-L877.

@Shrews
Copy link
Contributor

Shrews commented Jan 17, 2022

I tried to validate that we were actually using the image created from the new GHA job that creates the image for the integration tests. I expected PR #960 to fail the integration tests, but they did not (and it does seem the exception is being written to the proper location for the callback in the container image). It might be explained by the comment left by @AlanCoding.

I know @samdoran put a lot of work into this (thanks Sam!), but given that this doesn't seem to be testing the image created from the PR in the integration tests, I'm not on board with merging this.

@Shrews
Copy link
Contributor

Shrews commented Jan 18, 2022

I tried to validate that we were actually using the image created from the new GHA job that creates the image for the integration tests. I expected PR #960 to fail the integration tests, but they did not (and it does seem the exception is being written to the proper location for the callback in the container image). It might be explained by the comment left by @AlanCoding.

I know @samdoran put a lot of work into this (thanks Sam!), but given that this doesn't seem to be testing the image created from the PR in the integration tests, I'm not on board with merging this.

I believe that I've now found a way to make this work properly for integration tests. I'll be updating this PR.

@Shrews
Copy link
Contributor

Shrews commented Jan 18, 2022

Commit 08e3284 on PR #960 shows that the change I just pushed here should work properly for integration testing.

@Shrews Shrews merged commit 16cc1fe into devel Jan 18, 2022
@Shrews Shrews deleted the ci/gha branch January 18, 2022 17:20
Shrews pushed a commit to Shrews/ansible-runner that referenced this pull request Jan 18, 2022
Add GHA CI workflow. Includes sanity, unit, and integration tests.

Co-authored-by: David Shrewsbury <dshrewsb@redhat.com>
(cherry picked from commit 16cc1fe)
Shrews pushed a commit to Shrews/ansible-runner that referenced this pull request Jan 18, 2022
Add GHA CI workflow. Includes sanity, unit, and integration tests.

Co-authored-by: David Shrewsbury <dshrewsb@redhat.com>
(cherry picked from commit 16cc1fe)
ansible-zuul bot added a commit to ansible/project-config that referenced this pull request Jan 18, 2022
Remove ansible-runner project

ansible-runner is moving to using GHA for its CI (ansible/ansible-runner#944) and will not be using Zuul at all going forward.

Reviewed-by: None <None>
ansible-zuul bot pushed a commit that referenced this pull request Jan 20, 2022
[backport] [release_2.1] Create GitHub Actions workflow for running tests (#944)

Add GHA CI workflow. Includes sanity, unit, and integration tests.
Backport of #944
Co-authored-by: David Shrewsbury dshrewsb@redhat.com
(cherry picked from commit 16cc1fe)

Reviewed-by: Alexander Sowitzki <dev@eqrx.net>
Reviewed-by: None <None>
ansible-zuul bot pushed a commit that referenced this pull request Jan 20, 2022
[backport] [release_2.0] Create GitHub Actions workflow for running tests (#944)

Add GHA CI workflow. Includes sanity, unit, and integration tests.
Backport of #944
Co-authored-by: David Shrewsbury dshrewsb@redhat.com
(cherry picked from commit 16cc1fe)

Reviewed-by: Alexander Sowitzki <dev@eqrx.net>
Reviewed-by: None <None>
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.

None yet

3 participants