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

Add running tests as a separate job in GitHub Actions CI/CD #40

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

l1storez
Copy link
Collaborator

@l1storez l1storez commented Aug 29, 2022

#27 The test should be as a separate job in the ci.yml or as a separate file?

@l1storez l1storez requested a review from webknjaz August 29, 2022 15:51
@l1storez
Copy link
Collaborator Author

update: it's not a final version, I try to find out how to run tests from the directory

- name: Install dependencies
run: |
python -m pip install --upgrade pip
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be conditional. This file should always be present. If it's not, that's a bug. If it's deleted from the repo accidentally, this should fail and not be skipped silently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i remove 'if'

python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python -m pip install --upgrade pip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is normally not necessary, unless you're facing specific issues. Also, try not to dump all the commands in a single step because it's harder to process the test job run report visually then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

python-version: ["3.8", "3.9", "3.10"]

steps:
- uses: actions/checkout@v3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to always specify the name. Just like with Ansible — it's the best practice.

strategy:
fail-fast: false
matrix:
python-version: ["3.8", "3.9", "3.10"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, while you're on it, please integrate yamllint with the following configuration:

This will help you keep the style in the YAML files consistent. You can do this in a separate PR but an atomic commit in this one would be fine as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine, i try to do it

@webknjaz
Copy link
Member

webknjaz commented Sep 9, 2022

Maybe reference an issue this is related to?

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
@l1storez
Copy link
Collaborator Author

l1storez commented Sep 9, 2022

Maybe reference an issue this is related to?

What do you mean? Run tests from the directory?

@webknjaz
Copy link
Member

webknjaz commented Oct 4, 2022

What do you mean? Run tests from the directory?

No, have useful text in the PR description.

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