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 new test infrastructure #29

Merged
merged 23 commits into from
Jul 31, 2024
Merged

Conversation

santacodes
Copy link
Collaborator

@santacodes santacodes commented Jul 22, 2024

Separated nox sessions inside project and template.

  • For project : project-tests tests the units inside the pybamm_cookiecutter project, and template-tests tests the cookiecutter template for a proper generation.
  • For the template : once a user generates a project (lets call it a generated project), the generated project has two nox sessions for the tests, generated-project-tests which tests all the units inside the generated project and user-tests which executes the test cases added by a user for their respective code.

CI job to test the generated project

  • A new CI job to test the generated project is added in the workflow, which generates a project using the cookiecutter template and runs the tests contained in the generated project to test if the entry points and other units are working as intended.
  • CI workflow for the user to test their code has also been added to the template, along with doctests and coverage.

Note

The tests running inside generated project will fail at the last step as a hook is yet to be configured to initialise git inside the template using a post_gen_project script, I thought this PR was getting too long to be reviewed so I left that out for a separate one. Also I used pipx to run the tests inside the generated project because nox wasn't working when changing the working directory to the generated project.

Subtask #26

@santacodes santacodes closed this Jul 22, 2024
@santacodes santacodes deleted the newtestinfra branch July 22, 2024 17:13
@santacodes santacodes restored the newtestinfra branch July 22, 2024 17:13
@santacodes santacodes changed the title Added template Added new test infrastructure Jul 22, 2024
@santacodes santacodes reopened this Jul 22, 2024
@santacodes santacodes mentioned this pull request Jul 22, 2024
16 tasks
@Saransh-cpp
Copy link
Member

Nice! I'll get to this once the tests pass.

@santacodes
Copy link
Collaborator Author

Nice! I'll get to this once the tests pass.

The tests are failing because the git initialisation hooks are not added yet, I left it out since this PR was getting too long and was thinking of making a new PR for the hooks, do you want me to add them in this same PR?

@Saransh-cpp
Copy link
Member

I think it would be better to create and merge a PR for git initialization before this PR goes in/is reviewed.

@santacodes
Copy link
Collaborator Author

The tests are now passing after the git initialisation hooks are merged, I will be adding the patch as I mentioned here to flag out the hatch-vcs as an opt-in feature.

.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
cookiecutter.json Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/noxfile.py Show resolved Hide resolved
Co-authored-by: Agriya Khetarpal <[email protected]>
santacodes and others added 2 commits July 24, 2024 16:09
…g}}/{% if cookiecutter.backend == "hatch" %}_version.pyi{% endif %}

Co-authored-by: Saransh Chopra <[email protected]>
.github/workflows/test_on_push.yml Show resolved Hide resolved
.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
cookiecutter.json Outdated Show resolved Hide resolved
.github/workflows/test_on_push.yml Show resolved Hide resolved
.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
cookiecutter.json Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/noxfile.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/noxfile.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/noxfile.py Outdated Show resolved Hide resolved
Co-authored-by: Agriya Khetarpal <[email protected]>
@santacodes santacodes force-pushed the newtestinfra branch 3 times, most recently from 1c7e057 to e2e2896 Compare July 27, 2024 17:38
Copy link
Sponsor Member

@brosaplanella brosaplanella left a comment

Choose a reason for hiding this comment

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

I just have a small comment about the test duplication.

@santacodes santacodes mentioned this pull request Jul 30, 2024
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.

Thanks @santacodes, overall looks good.
Some minor suggestions. Should be good to merge after one final review from @Saransh-cpp.

{{cookiecutter.project_name}}/noxfile.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
hooks/post_gen_project.py Show resolved Hide resolved
@santacodes santacodes requested a review from arjxn-py July 31, 2024 11:48
.github/workflows/test_on_push.yml Outdated Show resolved Hide resolved
cookiecutter.json Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/pyproject.toml Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Thanks, @santacodes! This looks great now!

@santacodes santacodes merged commit 0e07267 into pybamm-team:main Jul 31, 2024
35 checks passed
santacodes added a commit that referenced this pull request Aug 1, 2024
This is a follow-up on PR #29, the template has been completely migrated
to `copier`.
@santacodes santacodes deleted the newtestinfra branch August 2, 2024 13:33
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