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

Sample gauge specs end up with Error: Tests failed, only in debug mode [Windows] #518

Open
srp-at-ema opened this issue Nov 23, 2020 · 17 comments

Comments

@srp-at-ema
Copy link

Describe the bug

Sample gauge specs end up with Error: Tests failed, only in debug mode

There is no html-report report: if I first Run the test then a report is made. When I then Debug it the output is not updated.

What command(s) did you run when you found the bug?

I click on Debug Spec above # Getting Started with Gauge

Note: After mkdir ptg, cd ptg and gauge init js and before running the spec, I open the ptg folder in vscode and do a npm init. I also reinstalled node LTS a couple days ago after a thorough clean.

Output, stack trace or logs related to the bug

Running tool: gauge run c:\Users\...\source\ptg\specs\example.spec --simple-console --hide-suggestion
Runner Ready for Debugging
D
ebugger listening on ws://127.0.0.1:19
18/20448ad4-c
446-492a-a
f0d-d2accd
c18610
F
or help, see: https://nodejs.org/en/docs/inspector
D
ebugger attached.
# Getting Started with Gauge
  ## Display number of items
  ## Must list only active tasks

Error: Tests failed.

Versions

Gauge (Output of gauge -v)

gauge -v
Gauge version: 1.1.5
Commit Hash: f455126

Plugins
-------
html-report (4.0.12)
js (2.3.14)
screenshot (0.0.1)

Node.js/Java/Python/.Net/Ruby version

node -v
v14.15.1

Operating System information

Get-ComputerInfo -Property os* | select OSName, OsArchitecture | fl

OsName         : Microsoft Windows 10 Pro
OsArchitecture : 64-bit

IDE information

code-insiders.cmd --version
1.52.0-insider
6026ab576dc6a4fbb8e255241a364816f42464c5
x64
@ElmaCat
Copy link

ElmaCat commented Nov 23, 2020

I've got the same error. Tests Failed in DEBUG mode and the browser never even open.

@zabil
Copy link
Member

zabil commented Nov 25, 2020

browser never even open.

This may be unrelated to this issue.

I see that the tests run and the browser launches but the output reports as test fails and the reports are not generated. This happens only on windows.

Screenshot 2020-11-25 at 15 21 13

@srp-at-ema
Copy link
Author

srp-at-ema commented Nov 27, 2020

It works with version 0.0.16 of the extension, not with version 0.0.17.

There has been a change in gaugeExecutor.ts between v0.0.16 and v0.0.17 that affects how the debugger handles a child process. And in that change Windows is treated differently than other platforms. As a result, ~\.vscode>extensions\getgauge.gauge-0.0.17\out\extension.js is missing detached:!0. When patching extensions.js the problem is resolved in v0.0.17.

Before:
this.childProcess=n.spawn(s,r,{cwd:t.getProject().root(),env:i})

After:
this.childProcess=n.spawn(s,r,{cwd:t.getProject().root(),env:i,detached:!0})

I don't know the motivation behind the change. In theory I would do a PR discarding the if statement.

The source code from gaugeExecutor.ts

v0.0.16:
this.childProcess = spawn(cmd, args, { cwd: config.getProject().root(), env: env, detached: true });

v0.0.17:

let options = { cwd: config.getProject().root(), env: env , detached: false};
if (platform() !== 'win32') {
     options.detached = true;
}
this.childProcess = spawn(cmd, args, options);

PS: this was found thanks to ElmaCat

@sriv
Copy link
Member

sriv commented Nov 30, 2020

Thanks for the detailed investigation @srp-at-ema and also thanks to @ElmaCat .

This change was introduced as part of this commit which we had to do to address certain scenarios that caused gauge (or the runner) process to dangle. Effectively this change moves gauge and the runners to a separate process group, and on exit it kills the entire process group.

This approach does not work on windows.

We'll investigate why this is causing this issue. At first glance, I believe by default options.detached is not set, so in theory, for windows it should be the same as not setting options.detached?

@srp-at-ema
Copy link
Author

in theory, for windows it should be the same

That's what it looks like, the option is missing from extension.js on Windows when setting detached: false in gaugeExecutor.ts. This causes the error. The error disappears when manually adding detached:!0 to extensions.js. It still not clear why win32 requires a separate treatment (unless you are counting on it to find out how many users are on that platform).

certain scenarios that caused gauge (or the runner) process to dangle

What scenarios? I have Windows and Linux at my disposal. I can give a hand at simulating those scenarios should you need that.

@sriv
Copy link
Member

sriv commented Nov 30, 2020

the option is missing from extension.js on Windows when setting detached: false in *gaugeExecutor.ts. *

Yes, I realized that too.

It still not clear why win32 requires a separate treatment (unless you are counting on it to find out how many users are on that platform).

We do not track usage. We used to, but that's history. The platform specific check is needed because of the way processes and process groups are handled in linux/macos v/s windows. In this context, the finding was that closing (or crashing) a parent process did not close/kill the subprocess. The process tree is IDE (vscode) -> gauge -> gauge runner.

@sriv
Copy link
Member

sriv commented Nov 30, 2020

I have Windows and Linux at my disposal. I can give a hand at simulating those scenarios should you need that.

thanks, that'll be great. Let me see if we have any platform specific issues that needs investigation, and I'll tag you there.

@srp-at-ema
Copy link
Author

Sorry "unless you are counting on it to find out how many users are on that platform" was a silly comment.

@srp-at-ema
Copy link
Author

This very interesting. And worth investigating as I have the feeling there is something at hand that can still be uncovered. I have not have enough exposure to working with such a call hierarchy in node or else, however fwiw I have the intuition this could work, if it was implement differently. Nevertheless, consider the opposite: what harm is there to use detached for all the platforms? Based on ElmaCat and mine findings it seems to be better, simply considering the error in question.

@srp-at-ema
Copy link
Author

I will use v0.0.16 for the time being: v0.0.17 introduced this issue and a series of library upgrades.

@sriv
Copy link
Member

sriv commented Nov 30, 2020

The issue is still very valid. reopening.

@sriv sriv reopened this Nov 30, 2020
@sriv
Copy link
Member

sriv commented Nov 30, 2020

what harm is there to use detached for all the platforms?

Please see options.detached docs. In non windows OS, node sets the subprocess in a separate process group, and makes it the leader there. This makes it easier to terminate the entire process tree. In windows such isn't the case.

FWIW - we implemented detached on all platforms by default, and then we discovered the above in our tests. So we had to resort to platform specific logic. It is never pretty, but the tradeoff of not doing this would lead to zombie processes that are much harder to identify and fix!

@srp-at-ema
Copy link
Author

Can you help me cause the dangling process?

@sriv
Copy link
Member

sriv commented Dec 1, 2020

Can you help me cause the dangling process?

I see it happening even in this case on windows i.e. after I see Error: Tests failed. when I run the example, I see html-report processes running indefinitely in the background.

@alex-gagnon
Copy link

I am seeing this same issue on Windows 10, Python, VS code, gauge v1.1.6. I can see all the reports, still getting Error: Tests failed in gauge execution console when running debug.

@sebor255
Copy link

sebor255 commented Jun 19, 2024

I am seeing this same issue on Windows 10, Python, VS code, gauge v1.1.6. I can see all the reports, still getting Error: Tests failed in gauge execution console when running debug.

I am seeing the same problem with Win 10, Python and VS code as Alex. When running a test with just 'run', it goes well, but in case I run it with debug it says: "Error: Tests failed." with no reason. The problem occurs at the line when I run a new Process (multiprocessing lib).


Gauge version: 1.6.7
Commit Hash: 684d870

Plugins

html-report (4.3.1)
json-report (0.4.0)
python (0.4.3)
screenshot (0.1.0)

Python 10.10, getgauge 0.4.3

@chadlwilson
Copy link
Contributor

There seem to be a number of problems with python debugging of various flavours right now, which may (or may not) have different root causes to the original post here. Do you have ability to try the same combination on MacOS or Linux and confirm your issue is Windows specific?

e.g.

getgauge/gauge-python#265
getgauge/gauge#2424

Im afraid I wouldnt expect this to be addressed quickly (or at all) unless someone volunteers to show the gauge-python plugin some love. 😓

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

No branches or pull requests

7 participants