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

chore: Run single-platform integration tests even if nightly build failed #2369

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

robin-aws
Copy link
Member

There's no good reason to not run the integration tests in this situation: the results can still help tell you if your PR is in good shape while the nightly build gets fixed.

Our previous attempt to fix this didn't work because if check-deep-tests failed, the integration-tests job was skipped no matter what it's if clause evaluated to. Instead this change removes the need dependency entirely. All jobs still have to pass to unblock the PR anyway.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@robin-aws robin-aws requested a review from fabiomadge July 6, 2022 18:15
@fabiomadge
Copy link
Collaborator

fabiomadge commented Jul 6, 2022

I think conserving resources for the people working on fixing the deep test issue is actually a good idea. Regarding the technical, I believe that this should work.

@@ -53,13 +53,12 @@ jobs:
run: dotnet pack --no-build dafny/Source/Dafny.sln

check-deep-tests:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'run-deep-tests') }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if: ${{ !contains(github.event.pull_request.labels.*.name, 'run-deep-tests') }}
if: !contains(github.event.pull_request.labels.*.name, 'run-deep-tests')

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that's what I tried first, but then the value looks like a YAML tagged value: https://yaml.org/spec/1.2.2/#691-node-tags :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

It worked before and the docs also claim so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure, that the initial problem weren't the unmatched parentheses?

Copy link
Collaborator

Choose a reason for hiding this comment

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

My bad, I just tried it. 😞

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.

None yet

2 participants