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

added coverage multiprocessing config for CI master builds #1312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jstucke
Copy link
Collaborator

@jstucke jstucke commented Dec 9, 2024

supersedes #1310

@jstucke jstucke self-assigned this Dec 9, 2024
@jstucke jstucke force-pushed the ci-master-multiproc-cov branch from 1408ba6 to 722b5b6 Compare December 9, 2024 14:51
@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.84%. Comparing base (ad4d6d8) to head (278c7dc).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1312      +/-   ##
==========================================
- Coverage   92.23%   91.84%   -0.39%     
==========================================
  Files         379      378       -1     
  Lines       23188    20967    -2221     
==========================================
- Hits        21387    19257    -2130     
+ Misses       1801     1710      -91     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jstucke jstucke force-pushed the ci-master-multiproc-cov branch 6 times, most recently from 4e23eb8 to 9a1d47e Compare December 10, 2024 14:22
@jstucke jstucke force-pushed the ci-master-multiproc-cov branch from 9a1d47e to 278c7dc Compare December 10, 2024 15:57
@jstucke jstucke marked this pull request as ready for review December 10, 2024 16:35
@maringuu
Copy link
Collaborator

Why not enable it in general?
Using sed to change the pyproject.toml does not seem like a good idea to me.

@maringuu
Copy link
Collaborator

One could e.g., change the CI script to run pytest as follows:

pytest \
    --cov=. \
    --cov-config=.github/.coveragerc

The .coveragerc would then contain any configuration that diverges from the pyproject.toml.

@jstucke
Copy link
Collaborator Author

jstucke commented Jan 16, 2025

Why not enable it in general? Using sed to change the pyproject.toml does not seem like a good idea to me.

Good question! This is more or less a hack. The simple reason is, that with the option multiprocessing enabled, CI builds take 6--8 minutes longer which is a bit too much (see e.g. https://github.com/fkie-cad/FACT_core/actions/runs/12179379759). For the nightly CI build this is not an issue, though.

@jstucke
Copy link
Collaborator Author

jstucke commented Jan 16, 2025

One could e.g., change the CI script to run pytest as follows:

pytest \
    --cov=. \
    --cov-config=.github/.coveragerc

The .coveragerc would then contain any configuration that diverges from the pyproject.toml.

Interesting proposal. I'm don't like the idea of using sed either (because it can easily break when the config file changes. I think the return value should indicate when it failed, though?). This on the other hand has the disadvantage that you need to maintain two config files henceforth, though (which shouldn't be that dramatic, since it only very rarely changes).

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.

3 participants