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: split tests for local / db [vol.2] #143

Merged
merged 6 commits into from
Feb 2, 2024
Merged

ci: split tests for local / db [vol.2] #143

merged 6 commits into from
Feb 2, 2024

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Feb 1, 2024

copy of #142, see: #142 (comment)

cc: @shaypal5

@shaypal5
Copy link
Collaborator

shaypal5 commented Feb 1, 2024

Thank you! Sorry for the bother.

@Borda
Copy link
Contributor Author

Borda commented Feb 1, 2024

Thank you! Sorry for the bother.

no problem, it can happen 🐰

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d19b3cf) 99.40% compared to head (d35a27b) 99.60%.

❗ Current head d35a27b differs from pull request most recent head 9db92d9. Consider uploading reports for the commit 9db92d9 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #143      +/-   ##
==========================================
+ Coverage   99.40%   99.60%   +0.19%     
==========================================
  Files           5        5              
  Lines         508      508              
  Branches       88       88              
==========================================
+ Hits          505      506       +1     
+ Misses          2        1       -1     
  Partials        1        1              

see 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d19b3cf...9db92d9. Read the comment docs.

@shaypal5
Copy link
Collaborator

shaypal5 commented Feb 1, 2024

pytest seems to be angry with the "NOT mongo".
The mongo marker is declared as required.
Maybe a lowercase "not"?
@Borda

@Borda
Copy link
Contributor Author

Borda commented Feb 1, 2024

pytest seems to be angry with the "NOT mongo".
The mongo marker is declared as required.
Maybe a lowercase "not"?

not sure I was using NOT without any issue, lets try it :)

.github/workflows/test.yml Outdated Show resolved Hide resolved
@Borda
Copy link
Contributor Author

Borda commented Feb 2, 2024

@shaypal5 seems some locals are still failing, mind have look, also feel free to push changes directly to this PR 🐿️

@shaypal5 shaypal5 changed the base branch from master to split-tests February 2, 2024 10:38
@shaypal5 shaypal5 merged commit a6e0871 into python-cachier:split-tests Feb 2, 2024
36 of 64 checks passed
@Borda Borda deleted the ci/split-db-local branch February 2, 2024 10:45
@shaypal5
Copy link
Collaborator

shaypal5 commented Feb 2, 2024

Ok, we're upping our game:

  1. I've pulled this into an origin dev branch with the name split-tests.
  2. I've invited you to the cachier organization and to the group of collaborators, which will give you a write access to this repo.
  3. I've protected the master branch, requiring an approved PR to merge into it (and disabling pushes to it, etc.).

I wanted to require some checks to pass before merging, and when trying to select one from the list, it made me realize the current matricization of the the test job across different platforms make Github consider them as a separate checks, which is what has been annoying you in the PR conversation. The upside is that all such builds run in parallel. Please take this into consideration when suggesting an alternative; perhaps this can be collapsed somehow just for the conversation view's sake.

Anyway, from now on, please work directly on dev branches of this repo and not your fork; you'll now be able to get your contributions tests directly against the live MongoDB server I've set up on the MongoDB Atlas platform on every push to your dev branches! :)

@Borda
Copy link
Contributor Author

Borda commented Feb 2, 2024

Anyway, from now on, please work directly on dev branches of this repo and not your fork; you'll now be able to get your contributions tests directly against the live MongoDB server I've set up on the MongoDB Atlas platform on every push to your dev branches! :)

Thank you and appreciate you granted me this privilege!
there is one last PR #140 coming from my fork, shall we keep it or port it?

@shaypal5
Copy link
Collaborator

shaypal5 commented Feb 2, 2024

Let's port it.
And there's already a PR waiting for your approval. :)

shaypal5 added a commit that referenced this pull request Feb 2, 2024
Authored-by: Borda <https://github.com/Borda> , Shay just merged this.
Co-authored-by: Jirka Borovec <[email protected]>
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.

2 participants