Test Runner fails to exit process when watch Plugin is included in tsconfig.

[FyndetNeo]

When i tried to use the cli of tstyche i noticed that the process never exited for me. I tried the cli on the examples of the Repo which worked fine.

After digging through the code i found that there was no explicit process.exit() call, so i added one inside the bin.js which fixed the problem.

I ran why from the package 'why-is-node-running' which revealed the culprit: a typescript plugin that im using is running in watch mode blocking the cli from exiting naturally.

It would also be possible to include the process.exit() at the end of the CLI.run method but i figured that would reduce flexibility for future uses.

I really like what you have created here and will definitely use this package more in the future.
Have a great day!

Looking forward to your feedback,
Leo

[mrazauskas]

Thanks for feedback.

There is no call to process.exit(), because of these lines from Node.js documentation:

Calling process.exit() will force the process to exit as quickly as possible even if there are still asynchronous operations pending that have not yet completed fully, including I/O operations to process.stdout and process.stderr.

In TSTyche the output is written to process.stdout and process.stderr. Hence no calling process.exit() felt like right thing to do. As documentation says:

The Node.js process will exit on its own if there is no additional work pending in the event loop.

---

Could you provide more details about the plugin you are using? Also: why it is "blocking the cli from exiting naturally"? On surface it sounds like the plugin is causing the problem. Why this can’t be fixed there?

[mrazauskas]

By the way, is that plugin needed for type testing? Perhaps it is worth to consider to exclude it from the tsconfig.json that includes the type test files? If that is possible.

[FyndetNeo]

Im happy to elaborate ^^

First of all, yes the plugin is quite important for the testing. Im using the graphQLSP plugin which provides an array of features, but the two i am using is autocompletion in queries and an analysis whether the queried fields are used in the file the query is in.

Because the Plugin is reporting the unused fields via the language server it is quite hard to access these diagnostics, as tsc does not include them on its own.

This is how i stumbled upon your package, though this is not the main intend of tstyche, it does work beautifully: Every issue reported by the typescript plugin is now testable in the terminal (which i needed to make it part of the pipeline).

I am too unexperienced in the lower levels of typescript development to tell whether or not the plugin should handle this issue on its own; it is however designed to run in the background. To give further inside here is the output of "why-is-node-running" (I modified the paths for privacy)

Output of "why-is-node-running"

The code used for logging:

import { Cli } from 'tstyche/tstyche';
import process from 'node:process';
import why from 'why-is-node-running';

const cli = new Cli();
await cli.run(process.argv.slice(2));

why();
process.exit();

the output:

# FILEHANDLE
(unknown stack trace)

# FILEHANDLE
(unknown stack trace)

# FILEHANDLE
(unknown stack trace)

# FSREQPROMISE
node_modules/tstyche/build/tstyche.js:1469 - entries = await fs.readdir(targetPath, { withFileTypes: true });
node_modules/tstyche/build/tstyche.js:1493 - await this.#visitDirectory(entryPath, testFilePaths);
node_modules/tstyche/build/tstyche.js:1493 - await this.#visitDirectory(entryPath, testFilePaths);
node_modules/tstyche/build/tstyche.js:1459 - await this.#visitDirectory(currentPath, testFilePaths);
node_modules/tstyche/build/tstyche.js:4076 - testFiles = await selectService.selectFiles();
scripts/checktypes.mjs:6                   - await cli.run(process.argv.slice(2));

# FSREQPROMISE
/users/hidden/projects/frontend/node_modules/@gql.tada/internal/dist/gql-tada-internal.js:2706 - var stat = (e, n = isFile) => d.stat(e).then(n).catch((() => !1));
/users/hidden/projects/frontend/node_modules/@gql.tada/internal/dist/gql-tada-internal.js:2736 - if (await stat(n)) {
/users/hidden/projects/frontend/node_modules/@gql.tada/internal/dist/gql-tada-internal.js:2753 - var n = await findTSConfigFile(e);
/users/hidden/projects/frontend/node_modules/@gql.tada/internal/dist/gql-tada-internal.js:3243 - var n = await loadConfig(e);
/users/hidden/projects/frontend/node_modules/@0no-co/graphqlsp/dist/graphqlsp.js:3142          - var o = await i.resolveTypeScriptRootDir(e.project.getProjectName()) || n.dirname(e.project.getProjectName());
/users/hidden/projects/frontend/node_modules/@0no-co/graphqlsp/dist/graphqlsp.js:3183          - })();
/users/hidden/projects/frontend/node_modules/@0no-co/graphqlsp/dist/graphqlsp.js:3185          - })(t, r, logger);

# TickObject
(unknown stack trace)

# TickObject
(unknown stack trace)

link to the plugin in question: https://github.com/0no-co/GraphQLSP

[FyndetNeo]

The solution of mine is incomplete as the macos build fails for a few tests. When testing locally the test cases succeed when adding a timeout of one second before exiting the process, which of course is not an option for an actual solution. There seems to be some operation of tstyche that is running past resolving of the cli.run promise.

I wouldn't mind using the stdout/stderr channel with a custom wrapper but if i want know whether the test completed without errors i would need to listen for an output including a hard coded string (like "Ran all test files.") which is also quite unstable.

Assuming graphqlsp is not at fault the possible solutions i see are:

• Diving deeper into the code to find the async operation that extends the promise of cli.run
• Write a small script that reads stdout/stderr and end the process manually on either a stdout containing "Ran all test files." with exit code 0 or a stderr with exit code 1 (This of course is just a solution for my project, not your repository)

Im open for any ideas or insight on this

[mrazauskas]

I took a quick look at failing tests and I see that the test runner is truncating the output. We talked about that with the author of the test runner yesterday. And it seems like the culprit is process.exit() calls in the test runner. See: #270 (comment)

---

It is very interesting how you are using TSTyche. I was playing with LSP plugins some time ago. Hm.. Somehow I was sure they simple respond to queries from language service. I can take a deeper look in a day or two.

If that is possible, could I ask to crate a minimal reproduction repo? It would be very useful how you put all the tools together.

[mrazauskas]

Another idea. In case if there is no way to make the plugin exit, I could add --forceExit option. Similar to the one in Rollup: https://rollupjs.org/command-line-interface/#forceexit

[FyndetNeo]

I have created a reproduction repository for you to try out: https://github.com/FyndetNeo/tstyche-process-exit-reproduction

When you pull it, run yarn and then run yarn tstyche you should see the warning from the App.tsx because of an unused field. Also the process does not exit.

fyi, the useQuery function is from Apollo but is unrelated to the issue, in the project i am working on we use a different approach for the actual querying and also do not utilise the type generation from gql.tada.

I see nothing wrong with implementing the process.exit behind a flag like rollup does, but i feel like nonetheless we should investigate the error that appears on macOS.
As i work on a macOS and can reproduce this issue locally i will try to take a deeper look at it in the next few days.
I can also implement the feature flag for the process.exit in this PR if you'd like.

Looking forward to contributing to this :D

[mrazauskas]

Thanks. I will take a look.

Hm.. I was thinking about LSP process. It isn't designed to exit. It is a long running process that may be force restarted or force closed. Plugins should not care to exit. Most probably.

In this light, the --forceExit option sounds like a feasible solution that may be useful with other plugins too.

Good question: how to implement it so that output wouldn't get truncated.

I will think about that. But first I will take deeper look at the plugin. Perhaps adding a simple .unref() somewhere could make it all work? That's just a naive idea.

[mrazauskas]

After a quick look I have noticed that the plugin keeps on writing something into ./src/generated/graphql-env.d.ts file. Run tstyche --watch and you will see how TSTyche is rerunning tests because of changes in this file.

Output of why-is-node-running also points to fs.readdir() calls. Which is used by TSTyche to discover test files.

Here are two changes I made. In tsconfig.json file: "tadaOutputLocation": "./.generated/graphql-env.d.ts" (or any location outside of src). In tstyche.config.json add: "rootPath": "./src". Like this TSTyche does not see graphql-env.d.ts and exist normally.

Is there a reason to keep graphql-env.d.ts inside the src directory?

Edit: This is not a solution. See next comment.

[mrazauskas]

Sorry, the suggestion above does not report the error anymore. So it does not work.

I think the tadaOutputLocation option is the culprit. If it is present, the plugin touches the specified file each time the language service is created. This is done each time TSTyche runs or reruns (in the watch mode) test files.

Actually./src/generated/graphql-env.d.ts.tmp is written first and it gets renamed to ./src/generated/graphql-env.d.ts. Since everything is async, seems like this trips up fs.readir() somehow and TSTyche does not exit.

I will dig into fs.readir() later to try to understand what is happening.

[FyndetNeo]

Actually i think this is not the case. In the original project another tool is used for type generation instead, the tool used is not a typescript plugin and is instead run in its own process. Tstyche does not exit even if that tool is not running.
The original project has undefined as tadaOutputLocation. The graphQLSP plugin sadly still tries to save an introspection file to the path "undefined" throwing an error. I have made a pull request in the graphQLSPs repository that simply only runs saveTadaIntrospection if the tadaOutputLocation is not undefined.

Now it still might be that this means graphQLSP is running unnecessary operations in the context of tadaOutputLocation being undefined.

[mrazauskas]

Interesting. What are you passing to the tadaOutputLocation option? If I remove the line, TSTyche finds the expected error, but @gql.tada throws:

TadaError: No available introspection format for "undefined" (expected ".ts" or ".d.ts")

The error is printed after output from TSTyche test run. As if it comes from some unhandled promise? Hard to say.

Hm.. I will try to draft --forceExit implementation. There might be other plugins that do not exit naturally. If that works for LSP, I think TSTyche should have a way to handle these situations.

[FyndetNeo]

Yes that error comes from graphQLSP trying to save the introspection file to the path "undefined" which is neither a .ts or .d.ts file causing the error. I have made a pull request to graphQLSP to fix this issue.

I think it would be in the interest of this package to be able to ensure that it will exit. But if there are potentially unfinished operations in tstyche this can lead to problems. From my understanding the dispatch of EventEmitter does not allow this as it does not await each event being run. I will try to make a draft this eveneing (Berlin Time) showcasing what i mean, each handleEvent function would need to be async. That means it would be a breaking change

[mrazauskas]

I was playing with async events some time ago. That was not working, because reporter depends sequence of the events. If I remember it right, this was main limitation why I kept sync events. That was long ago, so might be I am mixing up something.

---

Perhaps --forceExit could be a simple event handler similar to this one: https://github.com/tstyche/tstyche/blob/main/source/handlers/ExitCodeHandler.ts

It could set a timeout that calls process.exit(). The timeout gets reset after any event or after any output to stderr or stdout. If there is no output or no event for some time, call process.exit().

What do you think?

(Hm.. That does not work with the watch mode.)

[mrazauskas]

Ah.. If --forceExit is specified, the handler can be added before (or after?) this line:

tstyche/source/cli/Cli.ts

Line 140 in 03e802d

this.#eventEmitter.removeHandlers();

That should make the watch mode exit too. Also there is no need to check for events, because there are none anymore. Only wait for stale stderr and stdout and exit.

---

The option must be defined here (with OptionBrand.BareTrue if only a CLI flag is added or OptionBrand.Boolean for both config option and CLI flag):

tstyche/source/config/OptionDefinitionsMap.ts

Lines 73 to 78 in 03e802d

	{
	brand: OptionBrand.BareTrue,
	description: "Remove all installed versions of the 'typescript' package and exit.",
	group: OptionGroup.CommandLine,
	name: "prune",
	},

After adding the definition, run yarn build and yarn generate. That generate JSON schema for config file and types.

[mrazauskas]

Actually, if events are not taken into account this can simply be a private method on the Cli class.

[mrazauskas]

Just found root of the problem! It comes from gql.tada. They are using TypeScript's watchers from ts.sys.watchFile: https://github.com/0no-co/gql.tada/blob/d9d78a7fd280320b7450e26235ed05b1353396d7/packages/internal/src/loaders/sdl.ts#L64

Just follow the logic and in the else branch you will find fs.watch. These are created with persistent: false. This is similar to .unref() on timers. So these are unrefed, but ts.sys.watchFile are not. It is enough to remove the branch that installs ts.sys.watchFile watchers and process exits naturally.

---

@FyndetNeo Could I ask to try removing the branch with ts.sys.watchFile watchers? And to see if that fixes the issue?

[mrazauskas]

@FyndetNeo By the way, do you know why gql.tada check command does not report the error in your reproduction repo?

Reading the description it sounds like the error should be caught, but I can't make this work.

I was thinking that in this case it makes more sense to use gql.tada check instead of TSTyche. Because TSTyche is a test runner and it treats selected files as tests files. It is able to check types and handles language service plugins, of course. But from semantic perspective, TSTyche is a test runner. That's why gql.tada check sounded like a better choice, but why it does not catch the error?

[mrazauskas]

@FyndetNeo Is this still important for you? Could you take a look at my comment above, please?

[mrazauskas]

Let me know if this is still important. Since there is no reply for some time, I assume the issue is solved and it can be closed.

Also I think that gql.tada check might be more suitable to perform these checks. Perhaps it's worth reporting the issue to them.