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

Test cases are not generated because of uncovered process exit status #3

Closed
lppedd opened this issue Feb 7, 2024 · 9 comments
Closed

Comments

@lppedd
Copy link

lppedd commented Feb 7, 2024

spawnSync returns an object containing a status property.

The exit code of the subprocess, or null if the subprocess terminated due to a signal.

However you're only checking for 0.
I'm not sure what subprocess terminated due to a signal means, but that's the case I see on my laptop.

Probably it's because I'm using Node through Gradle? Who knows.

return output.status === 0;

@mike-lischke
Copy link
Owner

mike-lischke commented Feb 7, 2024

In the Linux world a process reacts to certain signals another process can send to it, to terminate it prematurely. Typical signals are SIGTERM and SIGKILL. And when a process is stopped by a signal it cannot have been successful. So, testing only for 0 in the status is correct either for a non-null termination and a signal terminated process.

Is 0 not returned in your tests? What is it instead, null/undefined?

@lppedd
Copy link
Author

lppedd commented Feb 7, 2024

It's strange tho. My output.status is null, and the generation of .g4 files is completed successfully.

@mike-lischke
Copy link
Owner

Tricky decision. If I accept null as successful execution it might happen that the generation continues even though there might have been a signal/error.

@lppedd
Copy link
Author

lppedd commented Feb 7, 2024

Something strange going on, the pid returned from spawnSync is 0. Is it normal?

@mike-lischke
Copy link
Owner

I don't know, sorry.

@lppedd
Copy link
Author

lppedd commented Feb 7, 2024

Looks like spawning processes still shows issues in Node.js.
See nodejs/node#33458 for example.

Could you switch to something else? Like for example execSync.

@lppedd
Copy link
Author

lppedd commented Feb 7, 2024

I've tested execSync and it works. Or I've set shell: true with spawnSync and it works.

@mike-lischke
Copy link
Owner

Looks like spawning processes still shows issues in Node.js. See nodejs/node#33458 for example.

Could you switch to something else? Like for example execSync.

I use spawnSync to stay close to the original Java code, which also included the generation result in the test output, so it needed errors and normal output for the checks. But now that the test generation is a separate step there's no need anymore to capture the output, except for the check that generation went successful. Important is also that errors from ANTLR4 are printed to the console, to be able to recognize and fix them.

@lppedd
Copy link
Author

lppedd commented Feb 7, 2024

I'd give the shell: true option a shot, comparing the speed/side effects.

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

No branches or pull requests

2 participants