teraslice icon indicating copy to clipboard operation
teraslice copied to clipboard

Reduce Overall Test Time by targetting

Open godber opened this issue 10 months ago • 4 comments

Explore options for reducing the test runtimes by only running tests for relevant sub-packages.

godber avatar Jun 09 '25 16:06 godber

This might help https://nx.dev/

godber avatar Jun 30 '25 22:06 godber

But changing to it might be a huge hassle.

godber avatar Jun 30 '25 22:06 godber

I've looked into a couple ideas and have a proposed solution. We should create a new sub command within scripts or just a script that can be ran before we trigger the the other tests. For example lets say we had a script that would somehow handle all the logic for what tests need to be ran and which ones don't. Our test.yaml ci file would look something like this:

  determine-tests-based-off-commit:
    runs-on: ubuntu-latest
    needs: compute-node-version-vars
    steps:
      - name: Check out code
        uses: actions/checkout@v4
      - name: Setup Node 24
        uses: actions/setup-node@v4
        with:
          node-version: 24
          cache: 'yarn'
      - name: Install and build packages
        run: yarn && yarn setup
      - name: Determine tests to run
        id: determine
        # The output of determine-tests would be a json object that looks like this:
          # {
          #   "unit": true,
          #   "e2e": true,
          #   "website": false
          # }
        run: |
          echo "Running determine-tests..."
          JSON=$(yarn determine-tests)

          LINUX_UNIT_TESTS=$(echo "$JSON" | jq -r '.unit')
          E2E=$(echo "$JSON" | jq -r '.e2e')
          WEBSITE=$(echo "$JSON" | jq -r '.webiste')

          echo "unit=$UNIT" >> $GITHUB_OUTPUT
          echo "e2e=$E2E" >> $GITHUB_OUTPUT
          echo "website=$WEBSITE" >> $GITHUB_OUTPUT

  linux-unit-tests:
    if: needs.determine-tests-based-off-commit.outputs.unit == 'true'
    runs-on: ubuntu-latest
    needs: [determine-tests-based-off-commit, compute-node-version-vars]
    ....


  e2e-k8s-tests:
    if: needs.determine-tests-based-off-commit.outputs.e2e == 'true'
    runs-on: ubuntu-latest
    needs: [compute-node-version-vars, verify-build, cache-docker-images, cache-asset-bundles, determine-tests-based-off-commit]
    ...

  e2e-k8s-v2-tests:
    if: needs.determine-tests-based-off-commit.outputs.e2e == 'true'
    runs-on: ubuntu-latest
    ....

  test-website:
    if: needs.determine-tests-based-off-commit.outputs.website == 'true'
    needs: [verify-build, determine-tests-based-off-commit]

As far as the logic of the script. It would:

  • center around looking at whats changed in the latest commit
  • determine what changes happened in what package
  • find the dependency tree
  • Classify what tests should be ran, (for example any change that leads to a change in teraslice would trigger e2e tests. But if its just a change in the e2e directory, it will only run e2e tests.)

Maybe it could also see if the changed file is of a certain extension. Such as typescript, and see if the addition is only comment related. To not count that package as something that has been changed. Basically we would just be setting up path rules for all the packages, and when changed, would be marked for testing.

We could start off very broad by only focusing on a website change, which would only run website tests and expand furthur overtime.

This is better than adding something to ts-scripts test and determining it there because we would still be standing up all the runners even though we may only need a couple of them.

sotojn avatar Jul 01 '25 22:07 sotojn

I have a few TODO's that I want to add here now that the initial implementation has been merged.

Move Logic from the scripts directory into ts-scripts

What we gain from this:

  • Testing exported functions to validate they are working correctly
    • This will become more important as we expand this logic further. For now it's simple because we only check if there are only docs changes. Eventually we can add more specific logic to each test type (Example: integration tests won't run in the case we only made a change to teraslice-cli`
  • Importing other libraries
    • We are limited to what node has to offer if we keep it as a one off script. For now this is okay

Don't run asset-cache and docker-cache jobs when e2e and integration tests are skipped

Right now we let these run regardless, but don't need to run in specific circumstances

Introduce logic that only checks the diff of the current push instead of the entire branch diff

Right now we are constantly checking the entire diff of the PR branch to master every time we push. This is a safe bet but inefficient. We should only be looking at changes since the last push to determine what tests we should run.

This gains us the feature of not running tests again from a change that has no impact on a given test.

Example:

We create a new branch called add-feature-to-teraslice with the goal of adding a new feature to teraslice.

I make a commit and it modifies several files in terafoundation and teraslice.

Files that are changed:

[
  packages/teraslice/src/index.ts,
  packages/terafoundation/src/someFile.ts

]

I push this change up then make a PR. This will result in all test being ran. I realize I didn't add docs so I make another commit that adds docs and push it up. The files changed since the last push would look like this:

[
  docs/overview.md,
  docs/somenewFile.md
]

We made no change that could possibly impact the test results of the previous push to the PR. The previous PR has already ran tests on the impactful changes.

Problem: What if the last push failed tests?

Solution: We can add logic to our script that will look at the conclusion of the previous workflow on the PR and check to see if it was successful or not. We can find this out using Githubs "list workflow runs" API.

Problem: That's great but sometimes we push more changes before the previous workflow has time to finish. How does that work?

Solution: The api has a lot of information we can leverage to see whats going on in the previous workflow. We can look at the status object and find out if the workflow is in_progress or not. If it's still working, we can sit and wait for its result.

Overall if:

  • The state of the previous push is successful
  • The current push has no impactful changes to specific test suites

We could safely assume that we don't need to re-run them and just run website tests. If not we just re-run all test and let it potentially fail again.

Add skip feature to commit messages

If we implement the above logic successfully we can look at the final commit before a push to the PR and parse it for things like [skip-unit], [skip-e2e], or [only-integration] to have further control of specific tests when testing in ci. Then add a logic that failes the all-test-passed job when these are the reason jobs were skipped.

sotojn avatar Jul 10 '25 00:07 sotojn