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

Add Codecov #57

Merged
merged 3 commits into from
May 17, 2024
Merged

Add Codecov #57

merged 3 commits into from
May 17, 2024

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented May 14, 2024

Description of changes:

@jngrad
Copy link
Member Author

jngrad commented May 14, 2024

The pyMBE.py script has 55% code coverage at the time of writing. See #58 for the features that require our attention.

If we could get every pyMBE community member to contribute unit tests for 5 features and invest no more than 20 min of developer time per unit test, we should be able to reach 90% code coverage in pyMBE.py in about a week. We don't need to write simulation scripts, just a few np.testing.assert_allclose() or pd.testing.assert_frame_equal() checks. Convenience scripts under the lib/ folder might require more effort, since they need either synthetic data or a minimal ESPResSo simulation script as input.

The report is available online. You can generate the code coverage report locally with the following commands:

make unit_tests COVERAGE=1
make coverage_html
firefox coverage/index.html

This draft PR will be rebased once all dependency issues are sorted out. It is currently designed around Codecov, because I'm already familiar with this service, but I am open to alternatives. For completeness, here are the main competing services: Codecov, Coveralls, Codacy. Here is a brief comparison of these services.

@pm-blanco
Copy link
Collaborator

pm-blanco commented May 14, 2024

@jngrad a million thanks for addressing this point. I will add this to our tomorrow's meeting schedule so we can start splitting the workload of developing the tests.

@jngrad
Copy link
Member Author

jngrad commented May 15, 2024

@pm-blanco While we wait for the Pylint PR to be merged, could you please give Codecov access to the pyMBE organization? Right now I'm using my own Codecov token, which means only the commits on my fork can upload coverage reports. Here is the official quick start page. You will need to follow step 1 using your GitHub account to sign up and establish access rights (the app needs to get read access to the organization and write access to write a green check mark in the commit metadata) and step 2 to store the secret token in the pyMBE-dev/pyMBE repository settings. That's it. The YAML file described in step 2 is already available in this PR. Steps 3-6 are already done in this PR too.

@pm-blanco
Copy link
Collaborator

@jngrad I belive it should be done now!

@jngrad jngrad force-pushed the codecov branch 2 times, most recently from 6892d44 to 8f4e608 Compare May 16, 2024 21:13
Copy link

codecov bot commented May 16, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@jngrad
Copy link
Member Author

jngrad commented May 16, 2024

The code coverage reports are now uploaded to https://app.codecov.io/gh/pyMBE-dev/pyMBE and written to the commit status (green checkmark or red cross). New features require a unit test, otherwise code coverage will drop and CI will fail.

Things to keep in mind:

  • for branches from this repository, the secret token is used, and uploads always succeed
  • for PRs from forks, the secret token isn't revealed for security reasons, and tokenless mode is used: uploads may fail
    • GitHub has a shared pool of free actions for tokenless users; when exhausted, wait for it to refill and re-run job
    • action log: Upload failed: {"detail":"Tokenless has reached GitHub rate limit. Please upload using a token: https://docs.codecov.com/docs/adding-the-codecov-token. Expected available in 358 seconds."}
    • using OpenID Connect probably would not help us here, because it requires a secret too (for implementation details: codecov-action@d42a336:src/buildExec.ts#L47-L54)
  • the samples job runs long tests and samples that reproduce paper results
    • runs biweekly at 6:20 AM to avoid rush hours: cron schedule 20 6 5,20 * *
      • scheduled workflows may start several minutes late if the rush hour is too strong, and might be cancelled if there isn't enough cloud capacity to absorb the backlog (quite rare, https://www.githubstatus.com/)
      • rush hour is every hour at 0 min, hence the extra 20 min
    • can be manually triggered
      • the manual action will only become visible in the Action tab once the workflow is part of the main branch!
      • I can confirm it works on branches, but I haven't tried on PRs (which are actually branches with a prefix)

@jngrad jngrad marked this pull request as ready for review May 16, 2024 21:31
@jngrad jngrad requested a review from pm-blanco May 16, 2024 21:31
Copy link
Collaborator

@pm-blanco pm-blanco left a comment

Choose a reason for hiding this comment

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

Looks excellent to me, thank you very much @jngrad. With this PR now we can proceed to develop more unit tests for pyMBE more efficiently. I added the information in this PR to our wiki so it can be consulted by all contributors.

@pm-blanco pm-blanco merged commit 37491e8 into pyMBE-dev:main May 17, 2024
3 checks passed
@jngrad jngrad deleted the codecov branch May 17, 2024 07:37
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.

Reduce feedback time from the testsuite
2 participants