Added tests for template generation #9

santacodes · 2024-05-31T17:00:38Z

Description

Added basic tests for template generation using pytest-cookies.
Added cookiecutter as a dependency and setup ___init.py__ for the src layout.

Saransh-cpp

Thanks, @santacodes! Looks good to me. Could you please add a minimal CI job to check if the tests are working fine?

arjxn-py

Thanks @santacodes, Good Start I must admit :)
Some comments below to propose some improvements

tests/test_project_generation.py

Co-authored-by: Arjun Verma <[email protected]>

santacodes · 2024-06-09T17:19:52Z

Thanks, @santacodes! Looks good to me. Could you please add a minimal CI job to check if the tests are working fine?

Sorry for the delay @Saransh-cpp, I was travelling. I added CI for the tests, created a nox session for testing the template generation, and tested it on my branch. Could you please review and look at it? Also, I added uv to the nox backend, which seems to be faster.

Saransh-cpp

Thanks, @santacodes! Few minor changes below -

noxfile.py

src/pybamm_cookiecutter/__init__.py

.github/test_on_push.yml

Saransh-cpp · 2024-06-10T11:16:11Z

TODO:

Update branch protection rule to include the new CI (status checks must pass before merging)

Co-authored-by: Saransh Chopra <[email protected]>

santacodes · 2024-06-10T14:13:08Z

TODO:

Update branch protection rule to include the new CI (status checks must pass before merging)

I think I can only create a branch rule allowing merges only after status checks when the status checks themselves have been merged to the main branch.

Saransh-cpp · 2024-06-11T15:37:31Z

Hi, don't worry about the rules. I'll set them up.

Saransh-cpp

This looks good to me, thanks @santacodes! I'll wait for @agriyakhetarpal and @arjxn-py's reviews.

arjxn-py · 2024-06-11T15:43:34Z

Hi, I'll review this in a couple of hours.

arjxn-py

Looks much better @santacodes 🙂, Some comments below for minor improvements, feel free to ping me again.
Before we merge, we would want that the workflow starts running on this same PR.
Plus, I really appreciate that we're also adapting to uv in this.
xref PyBaMM#3825

tests/test_project_generation.py

.github/test_on_push.yml

tests/test_project_generation.py

Co-authored-by: Arjun Verma <[email protected]>

.github/test_on_push.yml

pyproject.toml

noxfile.py

tests/test_project_generation.py

agriyakhetarpal · 2024-06-12T09:44:43Z

P.S. I'm sorry for the late review! I have been a bit out of touch, plus I noticed that Saransh and Arjun had already put in their reviews. I hope this is helpful.

Co-authored-by: Agriya Khetarpal <[email protected]>

santacodes · 2024-06-12T15:33:45Z

@arjxn-py can you please review my last commit, I actually modified those test cases completely because I think they're better in terms of assertions and we don't have to play with paths as pytest-cookies mostly takes care of it.
So one of the test cases tests template generation with default contexts from cookiecutter.json while the other tests template generation with a custom context passed into the bake() function. If you feel anything is wrong feel free to point it out so that I could revert it back if needed.

arjxn-py

Thanks @santacodes, the tests look perfect to me now.

arjxn-py · 2024-06-13T07:34:14Z

Before we merge, I'd just want to confirm if we can safely resolve the conversation about uv caching plus can you maybe link the CI run on the latest commit from your fork here?

agriyakhetarpal

This looks good to me as long as the tests pass – resolved the previous conversation. Thanks for adding this, @santacodes!

xrefs for uv's caching to look at later: astral-sh/uv#2231, yezz123/setup-uv#25, actions/setup-python#822, actions/setup-python#818

.github/test_on_push.yml

Co-authored-by: Agriya Khetarpal <[email protected]>

agriyakhetarpal · 2024-06-13T16:04:51Z

(I think we should disable the setting that dismisses reviews on further commits)

arjxn-py · 2024-06-13T16:05:19Z

I'm merging this as @santacodes needs to start working on entrypoints as well.
Although everything looks good but in case something fails we can look into it later as well

arjxn-py · 2024-06-13T16:05:59Z

(I think we should disable the setting that dismisses reviews on further commits)

Yes, I was going to mention that reviews were getting dismissed automatically

added tests for template generation

bc11f0e

Saransh-cpp requested changes Jun 4, 2024

View reviewed changes

Saransh-cpp requested review from agriyakhetarpal and arjxn-py June 4, 2024 14:20

arjxn-py reviewed Jun 5, 2024

View reviewed changes

tests/test_project_generation.py Outdated Show resolved Hide resolved

tests/test_project_generation.py Show resolved Hide resolved

tests/test_project_generation.py Outdated Show resolved Hide resolved

tests/test_project_generation.py Show resolved Hide resolved

santacodes and others added 3 commits June 9, 2024 13:22

Apply suggestions from code review

7055ba0

Co-authored-by: Arjun Verma <[email protected]>

added assertion logs

13e2251

added tests for template generation in ci

6475043

Saransh-cpp requested changes Jun 10, 2024

View reviewed changes

noxfile.py Outdated Show resolved Hide resolved

src/pybamm_cookiecutter/__init__.py Outdated Show resolved Hide resolved

.github/test_on_push.yml Show resolved Hide resolved

santacodes and others added 2 commits June 10, 2024 19:28

Apply suggestions from code review

bb87ed6

Co-authored-by: Saransh Chopra <[email protected]>

added push on main branch ci job

e645022

Saransh-cpp previously approved these changes Jun 11, 2024

View reviewed changes

Saransh-cpp requested a review from arjxn-py June 11, 2024 15:38

arjxn-py reviewed Jun 11, 2024

View reviewed changes

Apply suggestions from code review

bad7af8

Co-authored-by: Arjun Verma <[email protected]>

santacodes dismissed Saransh-cpp’s stale review via bad7af8 June 12, 2024 09:00

agriyakhetarpal reviewed Jun 12, 2024

View reviewed changes

Apply suggestions from code review

0e2683d

Co-authored-by: Agriya Khetarpal <[email protected]>

arjxn-py mentioned this pull request Jun 12, 2024

Added code of conduct #10

Merged

arjxn-py and others added 2 commits June 12, 2024 18:55

Merge branch 'main' into tests

db04463

modified test cases for bake()

2f51084

arjxn-py previously approved these changes Jun 13, 2024

View reviewed changes

Merge branch 'pybamm-team:main' into tests

feaf031

santacodes dismissed arjxn-py’s stale review via 58db762 June 13, 2024 13:28

santacodes force-pushed the tests branch from 58db762 to feaf031 Compare June 13, 2024 13:35

agriyakhetarpal previously approved these changes Jun 13, 2024

View reviewed changes

.github/test_on_push.yml Outdated Show resolved Hide resolved

Update .github/test_on_push.yml

0cacd05

Co-authored-by: Agriya Khetarpal <[email protected]>

arjxn-py dismissed agriyakhetarpal’s stale review via 0cacd05 June 13, 2024 15:49

arjxn-py approved these changes Jun 13, 2024

View reviewed changes

agriyakhetarpal approved these changes Jun 13, 2024

View reviewed changes

arjxn-py merged commit dca6946 into pybamm-team:main Jun 13, 2024

Uh oh!

Added tests for template generation #9

Added tests for template generation #9

Uh oh!

Conversation

santacodes commented May 31, 2024

Description

Uh oh!

Saransh-cpp left a comment

Choose a reason for hiding this comment

Uh oh!

arjxn-py left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

santacodes commented Jun 9, 2024

Uh oh!

Saransh-cpp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Saransh-cpp commented Jun 10, 2024

Uh oh!

santacodes commented Jun 10, 2024

Uh oh!

Saransh-cpp commented Jun 11, 2024

Uh oh!

Saransh-cpp left a comment

Choose a reason for hiding this comment

Uh oh!

arjxn-py commented Jun 11, 2024

Uh oh!

arjxn-py left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agriyakhetarpal commented Jun 12, 2024

Uh oh!

santacodes commented Jun 12, 2024

Uh oh!

arjxn-py left a comment

Choose a reason for hiding this comment

Uh oh!

arjxn-py commented Jun 13, 2024

Uh oh!

agriyakhetarpal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

agriyakhetarpal commented Jun 13, 2024

Uh oh!

arjxn-py commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arjxn-py commented Jun 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

agriyakhetarpal left a comment •

edited

Loading

arjxn-py commented Jun 13, 2024 •

edited

Loading