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

Change spec runner to exit with failure for focus: true #13653

Merged

Conversation

straight-shoota
Copy link
Member

This changes the behaviour of the spec runner: In focus mode, the exit code of successful runs will be overriden to 1 to prevent CI from silently running on a subset of tests due to an accidentally commited focus: true.

focus: true is typically used in interactive mode where the print output is most important for the user to understand if the spec run was successful. The failure status code should not have any effect in this use case.

The old behiavour can be regained by setting the environment variable SPEC_FOCUS_NO_FAIL=1. Then the exit code will be success (0) if all focused specs succeed.

Resolves #10989

@HertzDevil
Copy link
Contributor

This default behavior is not immediately obvious to the user, so either the spec runner should print a message upon exit from an otherwise successful run, or crystal spec --help should state this behavior.

@straight-shoota
Copy link
Member Author

focus: true already prints a message:

$ crystal eval 'require "spec"; it(focus: true) {}'
.

Finished in 90 microseconds
1 examples, 0 failures, 0 errors, 0 pending
Only running `focus: true`

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

This is somewhat of a breaking change. We should signal that, don't you think?

The change makes sense to me.

@beta-ziliani beta-ziliani added this to the 1.10.0 milestone Aug 28, 2023
@straight-shoota straight-shoota merged commit 63e2342 into crystal-lang:master Aug 29, 2023
51 of 52 checks passed
@straight-shoota straight-shoota deleted the feat/spec-focus-fail branch August 29, 2023 08:28
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add spec flag to fail when focus: true
3 participants