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 a test workflow #43

Closed
wants to merge 2 commits into from
Closed

Add a test workflow #43

wants to merge 2 commits into from

Conversation

kgaughan
Copy link
Owner

@kgaughan kgaughan commented Aug 24, 2024

Summary by Sourcery

Introduce a GitHub Actions workflow to automate linting and testing for the project on the master branch.

CI:

  • Add a new GitHub Actions workflow to run linting and tests on push and pull request events targeting the master branch.

Copy link
Contributor

sourcery-ai bot commented Aug 24, 2024

Reviewer's Guide by Sourcery

This pull request adds a new GitHub Actions workflow file to implement automated testing and linting for the project. The workflow is set up to run on push and pull request events for the master branch, as well as manual triggers.

File-Level Changes

Change Details Files
Implement a GitHub Actions workflow for automated testing and linting
  • Define workflow triggers for push and pull request events on the master branch
  • Include a manual trigger option (workflow_dispatch)
  • Set up a lint job using Ubuntu latest runner
  • Configure Rye setup with caching for the lint job
  • Add steps for running formatting checks and linters using Rye
  • Set up a test job using Ubuntu latest runner
  • Implement a matrix strategy for testing multiple Python versions (3.8 to 3.12)
  • Configure Rye setup for the test job
  • Add steps for installing specific Python versions and running tests using Rye
.github/workflows/tests.yml

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @kgaughan - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider specifying the Rye version (0.36.0) in the test job for consistency with the lint job. This ensures the same version of Rye is used across all jobs.
  • The use of Rye for Python environment management is an interesting choice. While it's a capable tool, it might be less familiar to some contributors. Consider documenting the reasons for choosing Rye and providing setup instructions for contributors who may be new to it.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.


test:
runs-on: ubuntu-latest
strategy:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Consider adding caching for the test job to improve CI performance.

You're using caching for the lint job, but not for the test job. Adding caching for the test job could significantly speed up your CI pipeline, especially for repeated runs.

    strategy:
      matrix:
        python-version: 
    steps:
      - uses: actions/checkout@v3
      - name: Set up Python ${{ matrix.python-version }}
        uses: actions/setup-python@v4
        with:
          python-version: ${{ matrix.python-version }}
      - uses: actions/cache@v3
        with:
          path: ~/.cache/pip
          key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
          restore-keys: |
            ${{ runner.os }}-pip-

@kgaughan kgaughan closed this Aug 24, 2024
@kgaughan kgaughan deleted the test-workflow branch August 24, 2024 14:49
@kgaughan kgaughan restored the test-workflow branch August 24, 2024 14:49
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.

1 participant