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

Not all tests in textlint have been run #1041

Closed
takmatsukawa opened this issue Jan 30, 2023 · 9 comments · Fixed by #1043
Closed

Not all tests in textlint have been run #1041

takmatsukawa opened this issue Jan 30, 2023 · 9 comments · Fixed by #1043
Labels
Type: Bug Bug or Bug fixes Type: Testing Adding missing tests or correcting existing tests

Comments

@takmatsukawa
Copy link

What version of textlint are you using?

4251c9e

What file type (Markdown, plain text, etc.) are you using?

What did you do? Please include the actual source code causing the issue.

I followed the Development Workflow
, added a new test in config-initializer-test.ts and npm run test in packages/textlint.

What did you expect to happen?

What actually happened? Please include the actual, raw output from textlint.

The test I added never failed.

I found mocha's .only() is used here. According to mocha's doc, It forces only its specified tests to be executed.
After removing the .only, some tests failed.

@azu azu added Type: Bug Bug or Bug fixes Type: Testing Adding missing tests or correcting existing tests labels Jan 30, 2023
@azu
Copy link
Member

azu commented Jan 30, 2023

Ah, It .only should beremoved.

@azu
Copy link
Member

azu commented Jan 30, 2023

It related with Dynamic Import + .ts

Dynamic import can not import .ts file by default.
Probably, loader: ts-node/esm is required.

@azu
Copy link
Member

azu commented Jan 31, 2023

We need to add https://github.com/lo1tuma/eslint-plugin-mocha for preventing this issue.

@azu
Copy link
Member

azu commented Jan 31, 2023

@azu azu closed this as completed in #1043 Jan 31, 2023
@takmatsukawa
Copy link
Author

Good work! Additionally, we can use mocha --forbid-only option.

Be mindful not to commit usages of .only() to version control, unless you really mean it! To do so one can run mocha with the option --forbid-only in the continuous integration test command (or in a git precommit hook).

The option will make the executions of tests containing .only fail.

@azu
Copy link
Member

azu commented Jan 31, 2023

It may difficutlt to use --forbid-only.
We want to only use --forbid-only on CI.

if process.env.CI 
  mocha --forbid-only 
else 
  mocha

.mocharrc.js will resolve it, but I not want to use dynamic config.
(It make migration difficult)

I want to write like this.

MOCHA_FORBIT_ONLY=1 mocha

Howevert, mocha --forbid-only is not bad at start point.

@azu
Copy link
Member

azu commented Jan 31, 2023

@takmatsukawa
Copy link
Author

takmatsukawa commented Jan 31, 2023

I found an example like this:

.mocharc.js

'use strict';

module.exports = {
  'forbid-only': Boolean(process.env.CI)
};

I'm not sure if it works in this repo.

@azu
Copy link
Member

azu commented Jan 31, 2023

.mocharrc.js will resolve it, but I not want to use dynamic config.
(It make migration difficult)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug or Bug fixes Type: Testing Adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants