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

Make running of the tests optional #11

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from
Open

Conversation

Radonirinaunimi
Copy link
Member

The following adds an input that switch on/off running of the tests before building the package.

@alecandido
Copy link
Member

alecandido commented Feb 20, 2023

This is already available, just undocumented: you can simply define an empty script to run for poe test, without the need of passing extra arguments.

In this sense, it follows the general trend I'm trying to enforce, i.e. favor project-local configuration files over arguments in workflows (to keep workflows simple, and make them more explicit).
What we could do, it's to start using a dedicated script for releasing, like poe test-release. But I'm not much in favor of it, since it should pass tests before releasing (otherwise you can manually release with twine, you don't need necessarily the workflow, or one of it's options to bypass some steps).

@Radonirinaunimi
Copy link
Member Author

I wanted to close this PR last week but luckily I didn't because I still wanted to discuss this a bit.

Could you may be expand on the following?

This is already available, just undocumented: you can simply define an empty script to run for poe test, without the need of passing extra arguments.

The reason why we'd need this feature now is when some extra-steps have to happen between the installation and testing (although I admit that this shouldn't be common). For example, now in nnusf since the data files are copied into the user directory (with appdirs) regardless of the package was installed using the development or pip-mode these data need to be fetched prior to running any commands:

https://github.com/NNPDF/nnusf/blob/2715753995ed44563180bf9a8afc91f30a24d0e7/.github/workflows/unittests.yml#L38-L39

Just curious but not relying on a reusable would also be fine.

@alecandido
Copy link
Member

You can perfectly do it without touching the workflow. Indeed, the workflow sets up some common context: Python, Poetry, install deps, install the package. And, eventually, it runs a script with:

poe test

Now, poe test invokes the script test defined in the pyproject.toml file, in the tools.poe.tasks table.
https://github.com/NNPDF/nnusf/blob/7c5972536a9d1b1c26db4309b0129e9ea1be52a3/pyproject.toml#L77
Now, this script is usually just a single command, but it doesn't have to. E.g. you can define:

[tools.poe.tasks]
fetch-data = "nnu get theory"
just-test = "pytest ..."
test = ["fetch-data", "just-test"]

Check https://github.com/nat-n/poethepoet#types-of-task for the full range of options :)

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.

2 participants