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 CI to run tests on pull requests #150

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

Conversation

mib1185
Copy link
Contributor

@mib1185 mib1185 commented Feb 12, 2022

Maybe the actions needs to be enabled first on https://github.com/kbr/fritzconnection/settings/actions

working example: mib1185#1

@kbr
Copy link
Owner

kbr commented Feb 13, 2022

Thanks, that's something I also have had in mind and wanted to cover in the next months. Seems you have been a bit faster.

@mib1185
Copy link
Contributor Author

mib1185 commented Feb 17, 2022

improved the workflow a bit, so that only the needed tox environment is run, based on the python-version to be tested.
I think for the first implementation this should be fine and a good starting point for maybe further future improvements of the CI

@kbr
Copy link
Owner

kbr commented Mar 8, 2022

Thank you for the work and the time you invested. I'm new to github workflow, therefore it took me also some time to dive into the basics. As far as I understand how it works, tox is not needed as a requirement – because tox does locally the job of github workflow. For a basic quickstart running automatic tests the following (mostly based on your configuration) should be feasible (docs and coverage excluded):

name: Tests

on:
  push:
    branches:
      - master
  pull_request: ~

jobs:
  tests:
    name: Python ${{ matrix.python-version }}
    runs-on: ubuntu-latest

    strategy:
      matrix:
        python-version: [3.6, 3.7, 3.8, 3.9, 3.10]
        
    steps:
        -uses: actions/checkout@master
        
        -name: Set up Python ${{ matrix.python-version }}
        uses: actions/setup-python@v2
        with:
            python-version: ${{ matrix.python-version }}
            
        -name: Install dependencies
        run: |
            pip install --upgrade pip
            pip install pytest requests
            
        -name: run tests (pytest)
        run: pytest

But I have to admit, that the @v2 in actions/setup-python@v2 is still unclear for me. Searching the docs reveals to something like "semantic versioning" what makes no sense in this context.

@mib1185
Copy link
Contributor Author

mib1185 commented Mar 13, 2022

@kbr I've changed the workflow, so that tox is not used anymore. If you would enable actions in https://github.com/kbr/fritzconnection/settings/actions (set "Allow all actions and workflows"), you will see the result here in this PR

@mib1185
Copy link
Contributor Author

mib1185 commented Apr 16, 2022

Hi @kbr do you think we can go ahead with this?

@kbr
Copy link
Owner

kbr commented Apr 16, 2022

Have to dive into this again: because of Ukrain and PyCon DE (yes, unlucky combination) I was engaged otherwise and have to find some timeslots for OSS in the next ...

@mib1185
Copy link
Contributor Author

mib1185 commented Apr 16, 2022

take your time and many thanks for your efforts 🙂

@mib1185
Copy link
Contributor Author

mib1185 commented Oct 1, 2022

Hi @kbr
do you mind we could bring this on the road, too? 🙂

Copy link

@Croydon Croydon left a comment

Choose a reason for hiding this comment

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

2024 calling. Maybe this can be moved forward? 🙂


strategy:
matrix:
python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
Copy link

Choose a reason for hiding this comment

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

Suggested change
python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
python-version: [""3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]

- uses: actions/checkout@master

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
Copy link

Choose a reason for hiding this comment

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

Suggested change
uses: actions/setup-python@v2
uses: actions/setup-python@v5

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

steps:
- uses: actions/checkout@master
Copy link

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@master
- uses: actions/checkout@v4

name: Build docs
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
Copy link

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@master
- uses: actions/checkout@v4

- uses: actions/checkout@master

- name: Set up Python 3.10
uses: actions/setup-python@v2
Copy link

Choose a reason for hiding this comment

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

Suggested change
uses: actions/setup-python@v2
uses: actions/setup-python@v5

name: Coverage
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
Copy link

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@master
- uses: actions/checkout@v4

- uses: actions/checkout@master

- name: Set up Python 3.10
uses: actions/setup-python@v2
Copy link

Choose a reason for hiding this comment

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

Suggested change
uses: actions/setup-python@v2
uses: actions/setup-python@v5

on:
push:
branches:
- master
Copy link

Choose a reason for hiding this comment

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

Why only on the master branch?

@Croydon
Copy link

Croydon commented May 16, 2024

But I have to admit, that the @v2 in actions/setup-python@v2 is still unclear for me. Searching the docs reveals to something like "semantic versioning" what makes no sense in this context.

The @v2 tells the CI to use the git object v2 of the referenced repository for the action. In this case, v2 is a git tag, but it could also be a branch or a git commit. You probably read something from semantic versioning because this particular actions uses semantic versioning for their versioning of their action.

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