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

[Dev] Include asset makers into CI tests #164

Closed
DanielYang59 opened this issue Jun 24, 2024 · 2 comments · Fixed by #195
Closed

[Dev] Include asset makers into CI tests #164

DanielYang59 opened this issue Jun 24, 2024 · 2 comments · Fixed by #195
Assignees
Labels
dx Developer experience enhancement New feature or request

Comments

@DanielYang59
Copy link
Collaborator

DanielYang59 commented Jun 24, 2024

Should include asset maker ipynb into CI tests.
Link to #62.

@DanielYang59 DanielYang59 added enhancement New feature or request dx Developer experience labels Jun 24, 2024
@DanielYang59 DanielYang59 self-assigned this Jun 24, 2024
@janosh
Copy link
Owner

janosh commented Jun 24, 2024

i briefly considered adding this in #163 via a new workflow file test-make-assets.yml but it would be slow and a waste of resources to run all these scripts on every commit and merge. ideally, the scripts should only when any of the code they rely on changed. e.g. make_assets/scatter.py would only run if it changed itself or if scatter.py changed. it's possible to check what changed with git diff. of course, the script could break due to changes in other files as well but I think the risk of that is not high enough to rerun the script every time

name: Test asset scripts

on:
  pull_request:
    branches: [main]
  push:
    branches: [main]
  workflow_dispatch:

jobs:
  run:
    runs-on: ubuntu-latest
    steps:
      - name: Check out repository
        uses: actions/checkout@v4

      - name: Set up Python
        uses: actions/setup-python@v5
        with:
          python-version: 3.9

      - name: Install package and dependencies
        run: pip install -e .

      - name: Run all make_assets scripts concurrently
        run: |
          scripts=$(find examples/make_assets -name '*.py')
          exit_code=0

          for script in $scripts; do
            echo "Running $script"
            python "$script" &
          done

          for job in $(jobs -p); do
            wait $job || let "exit_code+=1"
          done

          if [ "$exit_code" != "0" ]; then
            echo "One or more scripts failed. Please check the logs above for details."
            exit $exit_code
          fi

@DanielYang59
Copy link
Collaborator Author

Would be fixed by #195, thanks @janosh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer experience enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants