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

Mocha 8.3.1 breaks ability to add configuration to tail of it blocks #4598

Closed
4 tasks done
bcoe opened this issue Mar 8, 2021 · 2 comments · Fixed by #4599
Closed
4 tasks done

Mocha 8.3.1 breaks ability to add configuration to tail of it blocks #4598

bcoe opened this issue Mar 8, 2021 · 2 comments · Fixed by #4599
Labels
type: bug a defect, confirmed by a maintainer

Comments

@bcoe
Copy link
Contributor

bcoe commented Mar 8, 2021

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

In a few places in our codebases we configured retries and timeout using the following syntax:

 it('should delete note', () => {
    const output = execSync(`node deleteNote.js "${projectId}" "${noteId}" `);
    assert.include(output, `Note ${formattedNoteName} deleted.`);
    // Sometimes the delete note test is failing with the error:
    // Error: 5 NOT_FOUND: note with ID "test-note-${uuid}" for project
    // ${projectId} does not exist.
    // Attempting to work around this issue by retrying a few times.
    // DO NOT MERGE.  If this works, we should submit an upstream bug.
  }).retries(3);

In #4574 helpers like it() are refactored, such that they're wrapped in a closure and no longer have the attributes .retries(), .timeout(), etc.

Steps to Reproduce

Use mocha 8.3.1 to run the following block:

it('foo').retries(3);

Expected behavior: [What you expect to happen]

Settings can be chained on to it block.

Actual behavior: [What actually happens]

Exception occurs.

Reproduces how often: [What percentage of the time does it reproduce?]

Always happens.

Versions

Mocha 8.3.1.

Additional Information

One potential patch is returning the result of it.apply in the new closures that have been introduced.

@juergba juergba added type: bug a defect, confirmed by a maintainer and removed unconfirmed-bug labels Mar 9, 2021
@juergba
Copy link
Contributor

juergba commented Mar 9, 2021

@bcoe thanks for reporting this bug.

Your sample is not completely correct though, and I'm glad about this 😅.
It only breaks when you import or require the it() function.

import {it} from 'mocha';
it('foo').retries(3);

@bcoe
Copy link
Contributor Author

bcoe commented Mar 11, 2021

Your sample is not completely correct though, and I'm glad about this

@juergba ohhhh, that makes a lot of sense. I was surprised you didn't have more bug reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants