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

Switch from Travis CI to GitHub Actions for testing #175

Merged
merged 4 commits into from
Apr 24, 2021

Conversation

dataflake
Copy link
Member

Travis CI can be considered broken at this point. After they switched their business model and dropped the free unlimited use for Open Source communities they made it extremely hard to get enough free runs.

This PR moves all those test configurations into GitHub Actions: https://github.com/zopefoundation/ZEO/actions/workflows/tests.yml

@dataflake dataflake self-assigned this Apr 23, 2021
@jugmac00 jugmac00 self-requested a review April 23, 2021 20:44
Copy link
Member

@jugmac00 jugmac00 left a comment

Choose a reason for hiding this comment

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

LGTM! I had to restart CI twice to make all tests pass. We have some flaky tests.

@d-maurer
Copy link
Contributor

d-maurer commented Apr 23, 2021 via email

@d-maurer
Copy link
Contributor

d-maurer commented Apr 24, 2021

Jürgen Gmach wrote at 2021-4-23 13:45 -0700: @.*** approved this pull request.
LGTM! I had to restart CI twice to make all tests pass. We have some flaky tests.
There is a known race condition which should be fixed by #174.

Likely, I was wrong: the race condition test related to #174 comes with an upcoming (not yet released) ZODB version which almost surely is not used in the current setup.The failing tests you have observed likely indicate new race conditions. I documented one of them in "#176" (a trollius bug).

@navytux
Copy link
Contributor

navytux commented Apr 24, 2021

@dataflake, first of all thanks for caring about ZEO CI.

Do you think it could be possible to also add an entry into the matrix to verify ZEO against ZODB@master?

Doing so, in addition to verifying ZEO against ZODB release, will activate ZEO to run tests that reproduce data corruptions from #155 and #166 (see #167 (comment) for details), and it would also keep ZEO being tested for in-progress ZODB development.

Should we have such ZEO-ZODB@master coverage before, it is very likely that data corruption bug associated with #166 could be prevented from being introduced at all. The reason here is that bug started to appear only with ZODB 5.6, because during development of zopefoundation/ZODB#307 something was overlooked (details: #166 (comment)). Having ZEO-ZODB@master coverage would likely help to indicate that overlook with test failures, and would make developers to look into why things fail before ZODB 5.6 was released.

Thanks beforehand,
Kirill

/cc @d-maurer, @jamadden, @jmuchemb

Copy link
Contributor

@d-maurer d-maurer left a comment

Choose a reason for hiding this comment

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

Thank you for your quick action!

I do not yet know git actions at all and am thus likely not optimal to review this PR. But the known ZEO test jobs are run and able to detect problems. That means a big improvement.

@dataflake
Copy link
Member Author

@navytux I'll add that ZODB master test separately and will merge this now so it's available. Travis CI has failed us again and is not running at all now.

@dataflake dataflake merged commit 43a8246 into master Apr 24, 2021
@dataflake dataflake deleted the dataflake/gh_actions branch April 24, 2021 07:28
@navytux
Copy link
Contributor

navytux commented Apr 24, 2021

@navytux I'll add that ZODB master test separately

Thanks, @dataflake.

- name: Test
run: tox -e ${{ matrix.config[1] }}
- name: Coverage
if: matrix.config[1] == 'coverage'
Copy link
Member

Choose a reason for hiding this comment

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

I have not used gha with matrix, but I think this gets never true.

Sorry, just noticed this now as I saw we have an open issue about adding coverage, see #123

Copy link
Member Author

Choose a reason for hiding this comment

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

It is there for the moment when we have coverage testing.

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

Successfully merging this pull request may close these issues.

4 participants