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

feat: Add python tests script and run python tests in pre-push #390

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

cguedes
Copy link
Collaborator

@cguedes cguedes commented Aug 22, 2023

@gjreda I've just noticed that the #389 is failing in the CI after merge agains main!

This PR adds an utility to run python tests from the project root using yarn yarn python:tests and updates the git push hook to also run python tests.

@gjreda please fix the error reported in #391 (I don't know how 😅).


fixes #391

@cguedes cguedes changed the title Add python tests script and run python tests in pre-push feat: Add python tests script and run python tests in pre-push Aug 22, 2023
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #390 (dc731bc) into main (4516f86) will increase coverage by 3.19%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #390      +/-   ##
==========================================
+ Coverage   83.89%   87.09%   +3.19%     
==========================================
  Files         159       17     -142     
  Lines        9099     1046    -8053     
  Branches     1101        0    -1101     
==========================================
- Hits         7634      911    -6723     
+ Misses       1454      135    -1319     
+ Partials       11        0      -11     
Files Changed Coverage Δ
python/sidecar/projects.py 96.61% <100.00%> (ø)

... and 175 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@gjreda
Copy link
Collaborator

gjreda commented Aug 22, 2023

@cguedes The tests started to fail when you pushed this commit to the PR: f026603

I'll push a fix, but they were failing before merging into main. The root cause is that you changed the filename for projects storage, but did not update the tests.

@gjreda
Copy link
Collaborator

gjreda commented Aug 22, 2023

I suspect the bigger bug here is in our merge process: since we have historically ignored when codecov/patch is failing, I'm going to guess we simply thought it was codecov/patch that led to the ❌ on the PR, and not that actual tests were failing.

I know I've been guilty of this in the past too, so we should probably figure out how to make codecov/patch work as we'd like or simply get rid of it, since we do not trust it.

@cguedes
Copy link
Collaborator Author

cguedes commented Aug 22, 2023

I know I've been guilty of this in the past too, so we should probably figure out how to make codecov/patch work as we'd like or simply get rid of it, since we do not trust it.

You are right. I thought it was again the codecov/patch issue.
We need to fix our merge process and get rid of codecov/patch. @sehyod what do you think of this?

@cguedes cguedes marked this pull request as ready for review August 22, 2023 16:47
@gjreda gjreda self-requested a review August 22, 2023 16:47
@gjreda gjreda merged commit ba3c6f3 into main Aug 22, 2023
11 checks passed
@gjreda gjreda deleted the fix-python-unit-tests-filesystem branch August 22, 2023 16:47
@sehyod
Copy link
Collaborator

sehyod commented Aug 23, 2023

I know I've been guilty of this in the past too, so we should probably figure out how to make codecov/patch work as we'd like or simply get rid of it, since we do not trust it.

You are right. I thought it was again the codecov/patch issue. We need to fix our merge process and get rid of codecov/patch. @sehyod what do you think of this?

Yes, I 100% agree! codecov/patch is not super useful anyway, the most important is codecov/project

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.

bug: python tests don't run in the git push hook
3 participants