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

Run tests even if newsfile step fails #507

Merged
merged 3 commits into from
Mar 4, 2022

Conversation

DMRobertson
Copy link
Contributor

At present, if the newsfile step is skipped, then tests are also skipped. This happened on the main branch here:
https://github.com/matrix-org/sydent/runs/5306840738?check_suite_focus=true

At present, if the newsfile step is skipped, then tests are also skipped. This happened on the `main` branch here:
https://github.com/matrix-org/sydent/runs/5306840738?check_suite_focus=true
@DMRobertson DMRobertson requested a review from a team as a code owner February 23, 2022 16:44
@erikjohnston
Copy link
Member

FWIW Synapse repo does something like this, while also not running the tests if the newsfile lint fails. Not entirely sure how, but its mentioned at https://github.com/matrix-org/synapse/blob/develop/.github/workflows/tests.yml#L51-L57

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

I'm finding this confusing -- why would we want to run tests if the newsfile check fails?

@DMRobertson
Copy link
Contributor Author

Looks like there's a distinction between a step failing and a step being skipped. It's the newsfile check being skipped on main that I want to capture here. Will take Erik's advice and crib from Synapse.

@DMRobertson DMRobertson marked this pull request as draft February 25, 2022 11:33
@clokep
Copy link
Member

clokep commented Feb 25, 2022

I wonder if we can move those checks into the meta repo and re-use them? Not sure if that's possible though.

@DMRobertson DMRobertson self-assigned this Feb 25, 2022
@DMRobertson DMRobertson marked this pull request as ready for review March 3, 2022 12:32
@DMRobertson DMRobertson requested a review from clokep March 3, 2022 12:32
@clokep clokep requested a review from a team March 3, 2022 12:34
@DMRobertson DMRobertson merged commit 3d27f21 into main Mar 4, 2022
@DMRobertson DMRobertson deleted the dmr/run-tests-without-newsfile branch March 4, 2022 11:59
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