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.

Sign up for GitHub

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

Jump to bottom

feat: add option to not fail on failing test suite #4771

Merged
merged 6 commits into from
Jul 20, 2024
Merged

feat: add option to not fail on failing test suite #4771

merged 6 commits into from
Jul 20, 2024

Conversation

ilgonmic
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

Fixed #4216

Add possibility to distinguish 2 types of fails:

  • Problem with test run
  • Failing tests

When there are problems with test run, it is definitely problem.
When tests were run, but there are some test fails, it can be considered as ok.
It is useful to use Mocha in integrations with different build tools (for example Gradle), when we extract information from Mocha about tests and pass it to build tool and check exit code of Mocha.

Karma has similar feature

Alternate Designs

It can be done with 2 runs of Mocha:

  • with dry-run and check if JavaScript code can be run without problems
  • as usual - to run tests

But it requires 2 runs, which is not very convenient.

Why should this be in core?

When using Mocha and look at exit code, it is impossible to distinguish 2 cases

  • 1 failing test (exit code = 1)
  • Fail with running test (exit code = 1)

But these 2 cases can be completely different

Benefits

New useful feature for build pipelines integrations

Possible Drawbacks

Applicable issues

#4216

@ilgonmic ilgonmic changed the title Ilgonmic/fail on failing test suite Not fail on failing test suite Oct 18, 2021
@ilgonmic ilgonmic changed the title Not fail on failing test suite Add option to not fail on failing test suite Oct 18, 2021
@github-actions
Copy link
Contributor

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label Feb 16, 2022
@ilgonmic
Copy link
Contributor Author

Ping

@github-actions github-actions bot removed the stale this has been inactive for a while... label Feb 18, 2022
@github-actions
Copy link
Contributor

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label Jun 20, 2022
@github-actions github-actions bot closed this Jul 4, 2022
@JoshuaKGoldberg JoshuaKGoldberg removed the stale this has been inactive for a while... label Dec 2, 2023
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Dec 2, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

JoshuaKGoldberg
JoshuaKGoldberg requested changes Mar 4, 2024
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Looking good as a start! The logical flow makes sense and seems reasonable to me. 🔥

Requesting changes mostly on docs/naming and testing. A bit of code shuffling too, but I'm open to getting told I'm being overly nitpicky. 😄

lib/cli/run-helpers.js Outdated Show resolved Hide resolved
lib/cli/run.js Show resolved Hide resolved
test/integration/fixtures/require-undefined.fixture.js Outdated Show resolved Hide resolved
test/integration/options/failOnFailingTestSuite.spec.js Outdated Show resolved Hide resolved
lib/cli/run-helpers.js Outdated Show resolved Hide resolved
lib/cli/run.js Outdated Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg added status: waiting for author waiting on response from OP - more information needed semver-minor implementation requires increase of "minor" version number; "features" labels Mar 4, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title Add option to not fail on failing test suite feat: add option to not fail on failing test suite Mar 4, 2024
@JoshuaKGoldberg
Copy link
Member

👋 ping @ilgonmic, is this still something you have time for?

@JoshuaKGoldberg JoshuaKGoldberg added the stale this has been inactive for a while... label Jul 2, 2024
@ilgonmic
Copy link
Contributor Author

ilgonmic commented Jul 2, 2024

Hi @JoshuaKGoldberg
Sorry, I missed notifications from the review, I'll try to take a look on it on this week

@ilgonmic ilgonmic force-pushed the ilgonmic/fail-on-failing-test-suite branch from 5c1be0d to 964fa2c Compare July 15, 2024 10:21
@ilgonmic
Copy link
Contributor Author

@JoshuaKGoldberg Hi, I tried to fix all concers, could you please take a look?

@JoshuaKGoldberg JoshuaKGoldberg removed status: waiting for author waiting on response from OP - more information needed stale this has been inactive for a while... labels Jul 20, 2024
JoshuaKGoldberg
JoshuaKGoldberg approved these changes Jul 20, 2024
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

🙌 This looks great, thanks so much @ilgonmic!

I'll merge and release this in a new minor version soon. 🚀

docs/index.md Outdated Show resolved Hide resolved
lib/cli/run-helpers.js
return code => {
const clampedCode = passOnFailingTestSuite
? 0
: Math.min(code, 255);
Copy link
Member

Choose a reason for hiding this comment

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

[Praise] I like the moving of the Math.min(code, 255) here, nice spot!

lib/cli/run-helpers.js Show resolved Hide resolved
@rona-dinihari rona-dinihari mentioned this pull request Oct 24, 2024
Open
@abdulrahman305 abdulrahman305 mentioned this pull request Oct 27, 2024
Open
This was referenced Nov 1, 2024
Open
Open
Open
Open
@HaLaGu1L HaLaGu1L mentioned this pull request Nov 1, 2024
Open
This was referenced Nov 1, 2024
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Nov 9, 2024
Open
Open
@SkyeFull90 SkyeFull90 mentioned this pull request Nov 9, 2024
Open
This was referenced Nov 9, 2024
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
semver-minor implementation requires increase of "minor" version number; "features"
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🚀 Feature: Possibility to return 0 exit code when tests run successfully even with fails
3 participants
@ilgonmic @JoshuaKGoldberg @coveralls