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

Don't run tests for non-code changes #884

Closed
wd60622 opened this issue Jul 29, 2024 · 5 comments · Fixed by #898
Closed

Don't run tests for non-code changes #884

wd60622 opened this issue Jul 29, 2024 · 5 comments · Fixed by #898
Labels
docs Improvements or additions to documentation good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package maintenance

Comments

@wd60622
Copy link
Contributor

wd60622 commented Jul 29, 2024

Does it make sense to only run the test suite if the code has changed? If not running the tests if the case of only docstring changes is possible, is this desirable?

Something like this might be able to be done:

on:
  push:
    paths:
      - 'pymc_marketing/**/*.py'
  pull_request:
    paths:
      - 'pymc_marketing/**/*.py'
@wd60622 wd60622 added docs Improvements or additions to documentation question Further information is requested maintenance and removed question Further information is requested labels Jul 29, 2024
@wd60622 wd60622 changed the title Don't run tests for docstring changes Don't run tests for non-code changes Jul 29, 2024
@dandeandean
Copy link
Contributor

Hey @wd60622 as it happens I've been looking gh actions recently. We can see something similar in Github docs here, which run on any matching paths.

In this case it'd be:

on:
  pull_request:
    branches: [main]
    paths: '**.py'
  push:
    branches: [main]
    paths: '**.py'

Furthermore, you can add a paths-ignore, as described here. This may not be the way to go because it won't run if any of the paths match the pattern given.

@wd60622
Copy link
Contributor Author

wd60622 commented Jul 30, 2024

Hey @wd60622 as it happens I've been looking gh actions recently. We can see something similar in Github docs here, which run on any matching paths.

In this case it'd be:

on:

  pull_request:

    branches: [main]

    paths: '**.py'

  push:

    branches: [main]

    paths: '**.py'

Furthermore, you can add a paths-ignore, as described here. This may not be the way to go because it won't run if any of the paths match the pattern given.

Very nice @dandeandean
Do these file conditions happen at the file level or the individual level? I believe some of the same file's jobs target non-source code (notebooks, general formatting)

@wd60622
Copy link
Contributor Author

wd60622 commented Jul 30, 2024

What are your thoughts @juanitorduz
Seems like there are some prs that affect specfic elements of the repo. For instance, documentation / notebooks. Would saving 18+ mins of test suite help with dev experience?

@wd60622
Copy link
Contributor Author

wd60622 commented Jul 30, 2024

The ci.yml file has a few purposes:

  • Run pre-commit
  • Run tests (slow and fast) & check they both sucessfully run
  • Check for notebooks running

Running the pre-commit is fast and not much of a concern for me. Reasonable with notebooks too
The tests are the main concern.

Separating them out into test.yml and putting this conditional filter is where my head is at. Would like to hear other thoughts on this and if there are pros and cons to this approach.

@wd60622
Copy link
Contributor Author

wd60622 commented Jul 30, 2024

I've merged in #886 which uses the paths: ["pymc_marketing/**"] for generating the UML

@wd60622 wd60622 added the good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package maintenance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants