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

ci: set fail-fast as False to allow integration tests to continue on nightly Tutor failures #308

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

magajh
Copy link
Contributor

@magajh magajh commented Nov 26, 2024

Description

This PR updates the Tutor Integration Tests workflow to ensure that failures in the nightly version of Tutor do not cancel the execution of tests for stable versions (<18.0.0 and <19.0.0). fail-fast: false is added to the strategy, allowing the workflow to proceed even if nightly fails.

This change is important because the nightly version of Tutor is not stable and may occasionally fail due to issues unrelated to our tests. By implementing this update, we ensure that failures in nightly do not block critical tests for stable versions.

@magajh magajh requested a review from a team as a code owner November 26, 2024 12:21
@magajh magajh force-pushed the mjh/integration-tests-continue-on-nightly-failure branch from ba14453 to 45e66d4 Compare November 26, 2024 12:25
@magajh magajh changed the title ci(integration-tests): use continue on error for nightly version ci: allow integration tests to continue on nightly Tutor failures Nov 26, 2024
@magajh
Copy link
Contributor Author

magajh commented Nov 26, 2024

For more context on this solution, see eduNEXT/eox-tenant#232 (comment)

@magajh magajh marked this pull request as draft November 26, 2024 14:09
@magajh magajh changed the title ci: allow integration tests to continue on nightly Tutor failures ci: set fail-fast as False to allow integration tests to continue on nightly Tutor failures Nov 26, 2024
@magajh magajh marked this pull request as ready for review November 26, 2024 14:17
Copy link
Contributor

@BryanttV BryanttV left a comment

Choose a reason for hiding this comment

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

Thanks @magajh! This change affects all Tutor versions, right? For example, if v18 integration tests fail, do the other versions still run?

@magajh
Copy link
Contributor Author

magajh commented Nov 26, 2024

Thanks @magajh! This change affects all Tutor versions, right? For example, if v18 integration tests fail, do the other versions still run?

That's correct @BryanttV. I couldn't find a way to continue the execution for the other tutor_versions in the matrix only if 'nightly' fails. The use of continue-on-error marks the workflow for nightly as passed even if something fails during the execution

@BryanttV
Copy link
Contributor

@magajh, Could you try this? I don't know if it works

jobs:
  integration-test:
    name: Tutor Integration Tests
    runs-on: ubuntu-latest
    continue-on-error: ${{ matrix.experimental }}
    strategy:
      fail-fast: false
      matrix:
        tutor_version: ['<18.0.0', '<19.0.0']
        experimental: [false]
        include:
          - version: 'nightly'
            experimental: true

I was guided by this docs. However, if it doesn't work or we don't want to complicate things, it seems to me that the current solution is a good one.

@magajh
Copy link
Contributor Author

magajh commented Nov 26, 2024

@magajh, Could you try this? I don't know if it works

jobs:
  integration-test:
    name: Tutor Integration Tests
    runs-on: ubuntu-latest
    continue-on-error: ${{ matrix.experimental }}
    strategy:
      fail-fast: false
      matrix:
        tutor_version: ['<18.0.0', '<19.0.0']
        experimental: [false]
        include:
          - version: 'nightly'
            experimental: true

I was guided by this docs. However, if it doesn't work or we don't want to complicate things, it seems to me that the current solution is a good one.

@BryanttV Thanks for the suggestion! Since we’re only applying continue-on-error for the nightly version, I think we don’t need to introduce the additional experimental field in the matrix. That field could be useful for handling things dynamically across multiple versions, but in this case, looks like it adds unnecessary complexity.

Using continue-on-error: ${{ matrix.tutor_version == 'nightly' }} should be a simpler and more direct way to achieve the same result.

That said, one limitation of continue-on-error is that it marks the nightly tests as passed even if they fail, as it happened with the last commit dc479c7

@magajh
Copy link
Contributor Author

magajh commented Nov 27, 2024

@mariajgrimaldi @BryanttV @jignaciopm this is ready for the final review

mariajgrimaldi
mariajgrimaldi previously approved these changes Nov 27, 2024
BryanttV
BryanttV previously approved these changes Nov 27, 2024
Copy link
Contributor

@BryanttV BryanttV left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @magajh

@magajh magajh dismissed stale reviews from BryanttV and mariajgrimaldi via eab05b7 November 27, 2024 17:37
@magajh magajh force-pushed the mjh/integration-tests-continue-on-nightly-failure branch from e0a5db7 to eab05b7 Compare November 27, 2024 17:37
@magajh magajh force-pushed the mjh/integration-tests-continue-on-nightly-failure branch from eab05b7 to 5c1908e Compare November 27, 2024 17:40
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