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

Removing run-tests.py file. #4180

Merged
merged 72 commits into from
Jul 26, 2024
Merged

Conversation

prady0t
Copy link
Contributor

@prady0t prady0t commented Jun 14, 2024

Description

Trying to remove run-tests.py file.

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@prady0t
Copy link
Contributor Author

prady0t commented Jun 14, 2024

Yet to add conftest.py file

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.45%. Comparing base (ed412f8) to head (3075a77).
Report is 6 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4180   +/-   ##
========================================
  Coverage    99.45%   99.45%           
========================================
  Files          288      288           
  Lines        22089    22092    +3     
========================================
+ Hits         21969    21972    +3     
  Misses         120      120           

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

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.

This is looking good! I tried locally and

==================================================================== 44 passed in 100.46s (0:01:40) ====================================================================
nox > Session scripts was successful.

much faster than the serial implementation. I checked and the slowest scripts are run_ecm.py, experiment_drive_cycle.py, and pouch_cell_cooling.py. It might be worth improving those, but I guess 100 seconds for 44 scripts isn't too bad at all, afterall, and is rather great in terms of speedups.

noxfile.py Outdated Show resolved Hide resolved
tests/test_examples.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
prady0t and others added 11 commits June 15, 2024 00:03
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@prady0t
Copy link
Contributor Author

prady0t commented Jun 15, 2024

I think I messed up the branch. I'm having problems while pushing changes. I've made the changes for now.

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
prady0t and others added 2 commits July 10, 2024 12:10
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@agriyakhetarpal
Copy link
Member

@prady0t, could you fix the failure in the style jobs so that the tests can run?

prady0t and others added 3 commits July 10, 2024 15:15
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
…into removing-runtest

Solving merge conflicts.
@prady0t
Copy link
Contributor Author

prady0t commented Jul 10, 2024

@prady0t, could you fix the failure in the style jobs so that the tests can run?

Done. I removed lot of code lines from conftest.py file and now we are using -m for everything. If a user runs pytest without any marker value, it runs all the tests, otherwise only those specified by -m flag.

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
@prady0t
Copy link
Contributor Author

prady0t commented Jul 14, 2024

Added documentation fixes.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
tests/test_scripts.py Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
prady0t and others added 5 commits July 15, 2024 14:58
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
tests/test_scripts.py Outdated Show resolved Hide resolved
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
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 for your work here, @prady0t! This looks great now – just a couple of comments, and this is ready to go.

CONTRIBUTING.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@agriyakhetarpal
Copy link
Member

This will need approval from @arjxn-py since he previously requested changes, we can merge after that

prady0t and others added 2 commits July 26, 2024 23:12
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @prady0t.
Will just wait for CI to pass to observe logs once, then we can merge.

@agriyakhetarpal
Copy link
Member

@arjxn-py
Copy link
Member

No unnecessary logs this time around: https://github.com/pybamm-team/PyBaMM/actions/runs/10115401416/job/27976388229

Noticed that, just that we'd need to update branch to merge. My bad, I should have done it earlier.

@arjxn-py arjxn-py merged commit 48b4e48 into pybamm-team:develop Jul 26, 2024
23 of 24 checks passed
@kratman kratman mentioned this pull request Jul 26, 2024
js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024
* Removing run-test.py file

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Update test_examples.py

* Update noxfile.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Removing run-test.py file

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing example marker

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* adding conftest.py file

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* adding  back changes

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Update test_examples.py

* style: pre-commit fixes

* Removing run-test.py file

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removing redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Update test_examples.py

* Update noxfile.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* adding  back changes

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* removing loop for file discovery

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* removing loop for file discovery

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Adding more flags to pytest

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Adding markers for unit and integration tests

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Adding markers for unit and integration tests

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Added back assertDomainEqual

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Added fixture for debug mode

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* Removed style failures

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Removed style failures

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Update conftest.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* style: pre-commit fixes

* Update conftest.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update noxfile.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update pyproject.toml

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update tests/testcase.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update tests/test_scripts.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update tests/test_scripts.py

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* style: pre-commit fixes

* using -m instead of args

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* removed redundent files

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* fixing style failure

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* style: pre-commit fixes

* adding documentation changes

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Update CONTRIBUTING.md

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update CONTRIBUTING.md

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* adding suggestions

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* removing -s flag and getting file names in scripts log

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Update pyproject.toml

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

* Update CONTRIBUTING.md

Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

---------

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Co-authored-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjunverma.oc@gmail.com>
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
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.

None yet

4 participants