Update python_unit_test.yaml #28
Conversation
| @@ -7,6 +7,10 @@ on: | |||
| description: 'A URL identifying a Bash script that can run commands needed for service-specific test setup' | |||
There was a problem hiding this comment.
imo this repo-specific test file should be stored in the repo, not here
There was a problem hiding this comment.
I don't think there should be anything repo-specific. The whole idea was to make a configurable standard workflow. Install poetry, poetry install, run pytest. Maybe configure whether postgres is needed, maybe configure pytest command 🤷♂️
There was a problem hiding this comment.
maybe also optionally configure the working dir, for example in gen3authz the poetry files and tests files are in a subdir: https://github.com/uc-cdis/gen3authz/blob/dcef644/.travis.yml#L10
There was a problem hiding this comment.
So, I think there will be repo-specific steps for all users of this, just because the tests are all set up slightly differently across services. That's what the dependency-file-location argument is meant to enable. That being said, I wasn't sure on whether storing those here or in the repo makes more sense, so if you think putting that in the service repo will be a better idea, that sounds good to me.
There was a problem hiding this comment.
I have made this an optional item so you can add an input variable to a setup script but if it's not needed then we just skip.
There was a problem hiding this comment.
I also made Postgresql a optional input boolean.
There was a problem hiding this comment.
I'm not sure this URL will work reliably for the setup script on branches? Or I guess the person writing the workflow that uses this template will need to make sure the URL is dynamic based on where the PR is coming from
|
@Avantol13 @paulineribeyre I've updated based on your feedback. The changes to Fence to get the tests working are in this PR: uc-cdis/fence#1046. Could someone please take a look when they get the chance? |
There was a problem hiding this comment.
this looks good for fence, but i think we'll need more flexibility in configuration for other repos, as mentioned in this thread, such as:
- configure whether postgres is needed
- configure the working directory
- configure the pytest command so we can support coverage reports
…eat/reusable-unit-tests
added python version to default to latest currently 3.11 unless overriden as a variable in each repo Made the setup script location optional and an input for each repo In theory made postgresql service optional based on a boolean string for each repo
| @@ -0,0 +1 @@ | |||
| echo "No special setup needed! Continuing..." | |||
There was a problem hiding this comment.
this file can be deleted
… of file existence for the setup script
… they're said to exist before running
| bash ${{ inputs.setup-script }} | ||
| else | ||
| echo "Setup script ${{ inputs.setup-script }} not found." | ||
| fi | ||
| - name: Run test |
There was a problem hiding this comment.
| - name: Run test | |
| - name: Run tests |
This PR adds a reusable workflow that we can use to run unit tests for Python. It supports having a script passed as an argument for custom set up, testing multiple modules, and running multiple sets of tests. It also provides a built-in postgres database, for services that require one.