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

Error message when running cucumber test with npm prefix on Windows #2274

Closed
arbughiu opened this issue Apr 17, 2023 · 10 comments · Fixed by #2285
Closed

Error message when running cucumber test with npm prefix on Windows #2274

arbughiu opened this issue Apr 17, 2023 · 10 comments · Fixed by #2285
Assignees

Comments

@arbughiu
Copy link

👓 What did you see?

There is a bug causing "It looks like you're running Cucumber from a global installation." message to occur when the following conditions occur:

  • On a windows machine
  • Running a cucumber test using an npm script defined in package.json
  • Running the npm script using --prefix.

✅ What did you expect to see?

No error message should occur on Windows, if the --prefix directory is correct.

📦 Which tool/library version are you using?

latest

🔬 How could we reproduce it?

Please see an example of the bug that can be easily replicated here:
https://github.com/arbughiu/cucumber-windows-npm-prefix

@michael-lloyd-morris
Copy link
Contributor

Hmm... I'll look into this.

@michael-lloyd-morris michael-lloyd-morris self-assigned this Apr 18, 2023
@michael-lloyd-morris
Copy link
Contributor

Ok, I don't fully understand the way we are doing this check, but it looks a bit hackish. I found this library

https://www.npmjs.com/package/resolve-package-path

This might resolve both cases correctly, but do we want to add this package to fix this somewhat rare corner case?

@davidjgoss
Copy link
Contributor

To be clear this is only a warning, doesn't prevent you running. The detection is from https://www.npmjs.com/package/is-installed-globally and I don't think it's something we should implement/maintain in Cucumber. Since #2089 the handling for the issue that global (and other invalid) installations cause is much better, so arguably we could make this warning more subtle (one line, softer language) or even just remove it.

@arbughiu
Copy link
Author

@davidjgoss yes I agree its only a warning, and its not affecting behaviour, making this a minor bug. It's more of an annoyance right now, as the QAs running test scripts on Windows in my current company are asking me what they've done wrong to cause that warning message to occur. (the warning does not occur on Mac). Our setup is a bit more complex, but I've simplified it greatly in the example repo I provided.

@michael-lloyd-morris
Copy link
Contributor

If we remove it and there is a crash because cucumber was installed as a global, will the error message be clear? Could it be altered to make it clear in such circumstances? If so I would suggest doing that - remove this warning then alter the error message that arises when trying to import or require a step file when cucumber is installed globally.

That said, I have a feeling this might be very difficult to write a test case for.

@arbughiu
Copy link
Author

FYI I am not proposing we remove anything (as it would still be useful to be notified of a global installation) but rather fix the original intended behaviour.

In my sample repo we can see the following:

On Mac:
npm run test - runs fine ✅
npm --prefix . run test - runs fine ✅

On Windows:
npm run test - runs fine ✅
npm --prefix . run test - shows the error message above ❌

But npm --prefix . run test should produce the same result in both Windows and Mac.

The issue appears to be inside the is-installed-globally dependency

@michael-lloyd-morris
Copy link
Contributor

The issue is Windows.

And I'm saying this as a Windows user.

Kidding aside, the most direct fix would need to be in that is-installed-globally package. All we can do at the Cucumber level is find another approach.

@davidjgoss
Copy link
Contributor

Took the middle ground here - the warning remains but the language is softened and it's only shown in debug mode. The issues that stem from using a global install are much less cryptic than they used to be, so I think this is fine now.

#2285

@davidjgoss
Copy link
Contributor

(I think it would be worth raising an issue on is-installed-globally all the same though.)

@davidjgoss
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants