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

Using posargs to pass tests as arguments to tests nox session #4334

Merged
merged 6 commits into from
Aug 13, 2024

Conversation

prady0t
Copy link
Contributor

@prady0t prady0t commented Aug 11, 2024

Description

We can now pass specific test names to tests session. Example :
nox -s test -- tests/unit/test_discretisations/test_discretisation.py now only runs tests in test_discretisation.py

Copy link
Member

@agriyakhetarpal agriyakhetarpal left a comment

Choose a reason for hiding this comment

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

Thanks, @prady0t! Could you please document this in the Contributing guide? Edit: also, does this work with a specific test class/case as well, besides a test file?

Copy link

codecov bot commented Aug 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.46%. Comparing base (fdab3ef) to head (e7c5ed1).
Report is 159 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4334   +/-   ##
========================================
  Coverage    99.46%   99.46%           
========================================
  Files          289      289           
  Lines        22146    22146           
========================================
  Hits         22027    22027           
  Misses         119      119           

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

@prady0t
Copy link
Contributor Author

prady0t commented Aug 12, 2024

Thanks, @prady0t! Could you please document this in the Contributing guide? Edit: also, does this work with a specific test class/case as well, besides a test file?

Yes, it will work with a specific test class/case as well. You can try :
nox -s tests -- tests/unit/test_plotting/test_quick_plot.py::TestQuickPlot::test_simple_ode_model
Let me add documentations.

@agriyakhetarpal
Copy link
Member

I ran the command, and I think this is a nice improvement for running a selected piece of tests in an isolated virtual environment for local development. That said, it brings a lot of bottlenecks – installing and building PyBaMM is now taking longer than running a bunch of tests :) This might be a good motivator for switching to uv (#3825) sometime soon, both for CI and for our nox configuration.

@prady0t
Copy link
Contributor Author

prady0t commented Aug 12, 2024

I had an eye on that issue and wanted to work on it. Maybe we can also add it to the stretch goal?

@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented Aug 12, 2024

I remember @santacodes was working on it some time ago – if he agrees with your proposition, feel free to take over. :) That said, you both can collaborate and work on it together as well if you would like to, either by distributing tasks, preserving co-authorship on commits, or both.

@prady0t
Copy link
Contributor Author

prady0t commented Aug 12, 2024

I remember @santacodes was working on it some time ago – if he agrees with your proposition, feel free to take over. :) That said, you both can collaborate and work on it together as well if you would like to, either by distributing tasks, preserving co-authorship on commits, or both.

That would be nice!

@santacodes
Copy link
Contributor

I remember @santacodes was working on it some time ago – if he agrees with your proposition, feel free to take over. :) That said, you both can collaborate and work on it together as well if you would like to, either by distributing tasks, preserving co-authorship on commits, or both.

Sure, I would love to work with @prady0t, I already had some of the work done on one of my feature branches and then the work was stalled for some time if I remember correctly, I just have to resolve the merge conflicts, and open up a PR.

@prady0t
Copy link
Contributor Author

prady0t commented Aug 13, 2024

I've added the documentation. I think this is ready to be merged.

@arjxn-py
Copy link
Member

@agriyakhetarpal
Copy link
Member

Yes, seems to be random

@agriyakhetarpal agriyakhetarpal merged commit 9691d09 into pybamm-team:develop Aug 13, 2024
25 of 26 checks passed
santacodes pushed a commit to santacodes/PyBaMM that referenced this pull request Aug 16, 2024
…mm-team#4334)

* Using posargs to pass tests as arguments to  nox session

Signed-off-by: Pradyot Ranjan <[email protected]>

* style: pre-commit fixes

* Adding info to docs

Signed-off-by: Pradyot Ranjan <[email protected]>

* style: pre-commit fixes

---------

Signed-off-by: Pradyot Ranjan <[email protected]>
Co-authored-by: Pradyot Ranjan <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Arjun Verma <[email protected]>
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.

5 participants