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

Basic GitHub CI with pytest #5987

Merged
merged 12 commits into from Jun 3, 2022
Merged

Basic GitHub CI with pytest #5987

merged 12 commits into from Jun 3, 2022

Conversation

aaronsteers
Copy link
Contributor

@aaronsteers aaronsteers commented Jun 3, 2022

Adapted from @edgarrmondragon's recent work on the SDK: meltano/sdk#679

Bringing native GitHub pytest workflows, to allow us faster iteration.

  • Workflow setup time (installing poetry, pip, etc.): approx. 22 secons.
  • Pytest runtime: 7-10 min
  • Total runtime: approx 11 min

@aaronsteers aaronsteers changed the title Basic GitHub CI Basic GitHub CI with pytest Jun 3, 2022
@aaronsteers aaronsteers self-assigned this Jun 3, 2022
@aaronsteers aaronsteers marked this pull request as ready for review June 3, 2022 05:46
@aaronsteers
Copy link
Contributor Author

As of now, coverage files are not found. I'll drop for now, but would be great to reenable in future when we have time to debug:

https://github.com/meltano/meltano/runs/6721113729

image

@pandemicsyn
Copy link
Contributor

Exciting! @aaronsteers do you wanna disable the pytest on the gitlab side to cut down our build minute usage there or leave that enabled for a bit?


- name: Run pytest
run: |
poetry run pytest -v --cov-report= --cov meltano -m "$PYTEST_MARKERS"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@aaronsteers this is awesome! fwiw you need to run coverage with the --parallel flag so the .coverage* file is generated. Otherwise there's no artifact to upload (this error):

Warning: No files were found with the provided path: .coverage.*. No artifacts will be uploaded.
Suggested change
poetry run pytest -v --cov-report= --cov meltano -m "$PYTEST_MARKERS"
poetry run coverage run --parallel -m pytest -m "$PYTEST_MARKERS"

Similar to

https://github.com/meltano/sdk/blob/db90479f01d0426c380dd7d1b0234d4fbc9ec8e4/tox.ini#L17

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it might be worth computing coverage in all our supported Python versions and then combine them in a single report, like we do in the SDK:

https://github.com/meltano/sdk/blob/db90479f01d0426c380dd7d1b0234d4fbc9ec8e4/tox.ini#L47

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pandemicsyn re:

do you wanna disable the pytest on the gitlab side to cut down our build minute usage there or leave that enabled for a bit?

I think I'll hold off on removing those for now - just because I want to discuss the docker-image-dependent pytests and if we still need those. The pytest_fast could be dropped but I still like having that one as quick smoke check that doesn't take 20+ minutes to give a signal. I think we'll end up dropping them but want to make sure it's a thought-through decision.

@aaronsteers aaronsteers enabled auto-merge (squash) June 3, 2022 16:15
@netlify
Copy link

netlify bot commented Jun 3, 2022

🔮 Deploy Preview for meltano canceled.

Name Link
🔨 Latest commit 2cbb3b2
🔍 Latest deploy log https://app.netlify.com/sites/meltano/deploys/629a5bc280d0f1000813253f

- Use Python 3.9
- Do not use tox but raw Poetry instead
@aaronsteers aaronsteers merged commit eee011d into main Jun 3, 2022
@aaronsteers aaronsteers deleted the basic-github-ci branch June 3, 2022 19:30
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