bug: broken support for yarn executing tests #3482

jeripeierSBB · 2022-07-20T14:20:35Z

Prerequisites

I have read the Contributing Guidelines.
I agree to follow the Code of Conduct.
I have searched for existing issues that already report this problem, without success.

Stencil Version

2.17.1

Current Behavior

Executing yarn test with another script name as test fails.

Consider the following example specified in package.json scripts section:

...
 "scripts": {
    "test:dev": "stencil test --spec --e2e --coverage",
  },
...

if executing yarn test:dev it fails and the following output is generated:

jest args: test:dev --e2e --spec --coverage --max-workers=8
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/*/Desktop/test-fail-repro
  79 files checked.
  testMatch:  - 0 matches
  testPathIgnorePatterns: /Users/*/Desktop/test-fail-repro/.vscode, /Users/*/Desktop/test-fail-repro/.stencil, /Users/*/Desktop/test-fail-repro/node_modules, /Users/*/Desktop/test-fail-repro/www, /Users/*/Desktop/test-fail-repro/dist - 22 matches
  testRegex: (/__tests__/.*|(\.|/)(test|spec|e2e))\.[jt]sx?$ - 3 matches
Pattern: test:dev - 0 matches
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Expected Behavior

Tests should be executed as before version 2.17.1

Steps to Reproduce

npm init stencil
move to your directory
execute yarn
rename test to test:dev in package.json
executeyarn test:dev
See error

Code Reproduction URL

https://github.com/jeripeierSBB/stencil-yarn-fail-repro

Additional Information

No response

The text was updated successfully, but these errors were encountered:

alicewriteswrongs · 2022-07-20T16:46:07Z

Hello @jeripeierSBB, thanks for filing this issue! I was just able to validate that this is occurring. It look to be an issue specific to yarn, since in your reproduction case I am able to run the test:dev script with no problems w/ npm (i.e. npm run test:dev).

In any case, I'm going to add this to the backlog and hopefully we'll have a fix soon for running scripts like these w/ yarn!

jeripeierSBB · 2022-07-20T18:37:13Z

@alicewriteswrongs thanks for getting into this. I have to mention that with version 2.17.0 it worked, so this problem was newly introduced with version 2.17.1

alicewriteswrongs · 2022-07-20T18:39:11Z

That makes sense, this recent pr #3444 was included in the 2.17.1 release, so it makes sense that the behavior changed then.

alicewriteswrongs · 2022-07-20T18:41:48Z

edit: disregard this! commented on the wrong PR 🤦‍♀️

alicewriteswrongs · 2022-07-20T19:17:42Z

@jeripeierSBB would you mind providing some information about the version of yarn you're using, and how you had this working before upgrading to @stencil/core@2.17.1? I ask because in the reproduction repo that you provided I'm able to reproduce the issue with an earlier version of Stencil installed (2.13.0 to be specific). I'm using yarn 1.22.18 for testing locally.

manojreddymettu-pro · 2022-07-21T07:11:51Z

Hi, This issue is happening with NPM as well.

Worked fine till stencil/core:2.17.0 and after upgrading to 2.17.1 I see the below issue.

Note: The tests runs fine when you don't use explicit directory paths like --coverage-directory and --outputFile

Issue:

[19:08.9]  @stencil/core
[19:09.1]  v2.17.1 😊
[19:09.3]  testing spec files
[19:09.5]  jest args: --spec --coverage --json --coverage-directory=./dist-coverage/jest-test-coverage
           ./dist-coverage/jest-test-coverage --outputFile=.jest-test-results.json .jest-test-results.json
           --max-workers=8
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /Users/mettuman/IdeaProjects/proj-fork
  211 files checked.
  testMatch:  - 0 matches
  testPathIgnorePatterns: /Users/mettuman/IdeaProjects/proj-fork/.vscode,
  /Users/mettuman/IdeaProjects/proj-fork/.stencil, /Users/mettuman/IdeaProjects/proj-fork/node_modules, /Users/mettuman/IdeaProjects/proj-fork/www, /Users/mettuman/IdeaProjects/proj-fork/dist - 71 matches
  testRegex: (/__tests__/.*|(\.|/)(test|spec|e2e))\.[jt]sx?$ - 2 matches
Pattern: ./dist-coverage/jest-test-coverage|.jest-test-results.json - 0 matches

alicewriteswrongs · 2022-07-21T14:16:16Z

@manojreddymettu-pro I believe that is a separate issue, see #3481 and #3471

This removes some code from our argument parsing implementation which would look for an environment variable (`npm_config_argv`) set by npm and, if present, merge CLI flags found their into the ones already present on `process.argv`. We want to remove this for two reasons: - `npm_config_argv` is deprecated in `npm`, so in newer versions of the package manager it is no longer set and our code referencing it is effectively doing nothing - `yarn` v1.x still sets it (presumably for compatibility with `npm` that hasn't been removed yet, which causes an inconsistency between `npm` and `yarn` where certain `package.json` scripts will run without issue in `npm` but fail in `yarn` (see #3482) Accordingly, this commit deletes the offending function from `src/cli/parse-flags.ts` and makes a few related changes at call sites, etc which are necessary due to that change. This commit also refactors the spec file for `parse-flags.ts` to remove redundant tests. Because all of the supported CLI arguments (i.e. those added to `knownArgs`) are defined in `ReadonlyArray<string>` variables we can do a lot of exhaustive testing of all the arguments, set in the various possible permutations, etc, so we don't need to define a bunch of individual tests. Accordingly, the test file is refactored to remove redundant tests, add a few more test cases for the exhaustive tests, organize things a bit more with `describe` blocks, and ensure that we have good tests around all off the 'edge-case-ey' things.

This removes some code from our argument parsing implementation which would look for an environment variable (`npm_config_argv`) set by npm and, if present, merge CLI flags found their into the ones already present on `process.argv`. We want to remove this for two reasons: - `npm_config_argv` is deprecated in `npm` v7+, so in newer versions of the package manager it is no longer set and our code referencing it is effectively doing nothing - `yarn` v1.x still sets it (presumably for compatibility with `npm` that hasn't been removed yet, which causes an inconsistency between `npm` and `yarn` where certain `package.json` scripts will run without issue in `npm` but fail in `yarn` (see #3482) Accordingly, this commit deletes the offending function from `src/cli/parse-flags.ts` and makes a few related changes at call sites, etc which are necessary due to that change. This commit also refactors the spec file for `parse-flags.ts` to remove redundant tests. Because all of the supported CLI arguments (i.e. those added to `knownArgs`) are defined in `ReadonlyArray<string>` variables we can do a lot of exhaustive testing of all the arguments, set in the various possible permutations, etc, so we don't need to define a bunch of individual tests. Accordingly, the test file is refactored to remove redundant tests, add a few more test cases for the exhaustive tests, organize things a bit more with `describe` blocks, and ensure that we have good tests around all off the 'edge-case-ey' things.

alicewriteswrongs · 2022-07-22T20:42:41Z

@jeripeierSBB to update you, we merged a PR today (#3486) which I believe fixes the underlying issue you're running in to here.

If you want to test it out to make sure it fixes your issue you could:

clone the repo locally
do npm install, npm run build and npm pack
try installing the resulting .tgz file in our project by doing npm install ./path/to/stencil.tgz

If you'd rather wait for a pre-release or release build to try that out no worries, we will be doing one soon.

Thanks again for reporting this issue!

jeripeierSBB · 2022-07-25T13:39:17Z

providing some information about the version of yarn you're using, and how yo

@alicewriteswrongs Thanks for all your work on it and sorry for my late response. I'm using yarn v1.22.19, and in my project it really worked with stencil 2.17.0.

I tested your fix and it works with yarn, thank you! Should I close the issue myself?

alicewriteswrongs · 2022-07-25T14:56:09Z

Glad the fixed worked for you! Thanks again for reporting the issue, it pointed the way to a problematic little bit of code that was good to eliminate. As for the issue, we can leave it open until the fixed is released just so that it will continue to show up as open for anyone else who experiences the problem on a current stencil release.

…

On Mon, Jul 25, 2022 at 9:40 AM Jeri Peier ***@***.***> wrote: providing some information about the version of yarn you're using, and how yo @alicewriteswrongs <https://github.com/alicewriteswrongs> Thanks for all your work on it and sorry for my late response. I'm using yarn v1.22.19, and in my project it really worked with stencil 2.17.0. I tested your fix and it works with yarn, thank you! Should I close the issue myself? — Reply to this email directly, view it on GitHub <#3482 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPLRHBAWKBEEXPIAGFUI6TVV2KNVANCNFSM54D3REVQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

rwaskiewicz · 2022-08-01T16:46:22Z

The fix for this issue has been officially released as a part of Stencil v2.17.2. As a result, I'm going to close this issue and mark it as completed. If the bug re-appears, please feel free to open a new issue!

ionitron-bot bot added the triage label Jul 20, 2022

jeripeierSBB mentioned this issue Jul 20, 2022

chore(deps): update dependency @stencil/core to v2.17.2 sbb-design-systems/lyne-components#1288

Merged

1 task

alicewriteswrongs self-assigned this Jul 20, 2022

alicewriteswrongs added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label Jul 20, 2022

ionitron-bot bot removed the triage label Jul 20, 2022

alicewriteswrongs mentioned this issue Jul 22, 2022

fix(cli): remove usage of deprecated npm env var from arg parser #3486

Merged

14 tasks

rwaskiewicz closed this as completed Aug 1, 2022

This issue was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bug: broken support for yarn executing tests #3482

bug: broken support for yarn executing tests #3482

jeripeierSBB commented Jul 20, 2022

alicewriteswrongs commented Jul 20, 2022

jeripeierSBB commented Jul 20, 2022

alicewriteswrongs commented Jul 20, 2022

alicewriteswrongs commented Jul 20, 2022 •

edited

Loading

alicewriteswrongs commented Jul 20, 2022

manojreddymettu-pro commented Jul 21, 2022

alicewriteswrongs commented Jul 21, 2022

alicewriteswrongs commented Jul 22, 2022

jeripeierSBB commented Jul 25, 2022

alicewriteswrongs commented Jul 25, 2022 via email

rwaskiewicz commented Aug 1, 2022

bug: broken support for yarn executing tests #3482

bug: broken support for yarn executing tests #3482

Comments

jeripeierSBB commented Jul 20, 2022

Prerequisites

Stencil Version

Current Behavior

Expected Behavior

Steps to Reproduce

Code Reproduction URL

Additional Information

alicewriteswrongs commented Jul 20, 2022

jeripeierSBB commented Jul 20, 2022

alicewriteswrongs commented Jul 20, 2022

alicewriteswrongs commented Jul 20, 2022 • edited Loading

alicewriteswrongs commented Jul 20, 2022

manojreddymettu-pro commented Jul 21, 2022

alicewriteswrongs commented Jul 21, 2022

alicewriteswrongs commented Jul 22, 2022

jeripeierSBB commented Jul 25, 2022

alicewriteswrongs commented Jul 25, 2022 via email

rwaskiewicz commented Aug 1, 2022

alicewriteswrongs commented Jul 20, 2022 •

edited

Loading