Skip to content

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.

3 participants