Add CI workflow #12
Add CI workflow #12
Conversation
Add workflow that builds, run tests and lint on the repo. This is initiated by a push or pull request to the develop branch.
| - name: Set up Python 3.11 | ||
| uses: actions/setup-python@v3 | ||
| with: | ||
| python-version: "3.11" |
There was a problem hiding this comment.
Good idea to set this to match our Jenkins set up on our mirror repo on GitLab.
Is there a way to have the GitHub Actions workflow pick up the Python version set in the project config? This may be a pipe dream, but it feels like it could be useful to have the CI pipeline run on whatever we set as the lowest compatibility of Python in the project config.
No problem if you don't have time to look into this. I can pick it up.
There was a problem hiding this comment.
No worries, I have implemented this. The version flow is as follows:
- The Python version can be specified using the workflow_dispatch input python-version, on manual runs.
- If not specified, it will be read from pyproject.toml, using the lowest compatibility.
- In case of no specification found, will default to 3.11.
| pip install flake8 pytest | ||
| if [ -f requirements.txt ]; then pip install -r requirements.txt; fi | ||
| - name: Set PYTHONPATH | ||
| run: echo "PYTHONPATH=${{ github.workspace }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Do we need to set the PYTHONPATH here? Which bit of the workflow fails if we don't have it?
There was a problem hiding this comment.
We do need PYTHONPATH setup, or else pytest fails as it cannot import robota-core packages while running. I added this as a fix during prior testing.
There was a problem hiding this comment.
I had the same problem when working on this issue (I didn't create a PR as this was created already). My fix was using python -m pytest rather than calling pytest. This meant PYTHONPATH was no longer required as it ensures the tests are in the current python enviroment. Maybe this could be used instead?
There was a problem hiding this comment.
Thanks very much @Nigel007 - one of the delays in my next review of this PR was that I was trying to figure out why none of our other Python GA workflows needed the PYTHONPATH like this.
I think I'd prefer to go with the solution suggested by @Nigel007 , even though @lourencofsilva 's solution is also plausible, so that the GA workflow set up matches the dev environment set up as closely as possible (where we don't set PYTHONPATH explicitly either).
There was a problem hiding this comment.
I agree with this, python -m pytest is the best way of going about this. Thanks for the input @Nigel007. I have just altered this and removed the PYTHONPATH setup.
Add workflow_dispatch manual trigger, with option of specifying a python version. Default to minimum required version in pyproject.toml. Update setup-python to v5. Testing lint with ruff.
|
Have you considered adding mypy for type checking? It could help catch potential type-related issues early on, especially since the codebase already utilises the typing module. Just a thought! |
This is a good suggestion, @Nigel007 - we have used mypy in other projects and found it useful. Maybe you could add this in a separate issue/PR? I'd like to merge this PR as soon as possible, since @lourencofsilva created it some time ago and I've already delayed it for a while. |
There was a problem hiding this comment.
This is getting really close now, @lourencofsilva . Thanks so much for your great work on this.
| pip install flake8 pytest | ||
| if [ -f requirements.txt ]; then pip install -r requirements.txt; fi | ||
| - name: Set PYTHONPATH | ||
| run: echo "PYTHONPATH=${{ github.workspace }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Thanks very much @Nigel007 - one of the delays in my next review of this PR was that I was trying to figure out why none of our other Python GA workflows needed the PYTHONPATH like this.
I think I'd prefer to go with the solution suggested by @Nigel007 , even though @lourencofsilva 's solution is also plausible, so that the GA workflow set up matches the dev environment set up as closely as possible (where we don't set PYTHONPATH explicitly either).
Altered GA workflow so that PYTHONPATH is not setup, and run tests through `python -m` instead of directly calling `pytest`
There was a problem hiding this comment.
I think I'm happy with this now, @lourencofsilva - thanks. It took me a while to convince myself that the ${{...}} were safe but I think they are, provided I keep an eye on PRs that change the pyproject.toml file (which I'd have to do anyway).
Thanks again for your hard work on this.
Add workflow that builds, run tests and lint on the repo. This is initiated by a push or pull request to the develop branch. Resolves #11