resolves #99 pass version to the worker

[ggrossetie]

resolves #99

[ggrossetie]

I've added an explicit timeout on string-limit-2.test.js and event.test.js because tests are running forever on GitHub actions. I thought it was related to the Node version and/or OS but I don't see any pattern. I think one of our tests is just flaky. Hopefully, with the timeout we will get a proper error.

[ggrossetie]

Still running forever 😬
In theory, the default timeout is 30 seconds: https://node-tap.org/docs/cli/

[mcollina]

lgtm

[ggrossetie]

Thanks for your review @mcollina

I think the timeout in .taprc is in seconds (not milliseconds) so the actual timeout is 60000000 ms. When there's a failure, you can see the following in the log:

  ---
  env: {}
  file: test/thread-management.test.js
  timeout: 60000000
  command: C:\hostedtoolcache\windows\node\16.17.0\x64\node.exe
  args:
    - test/thread-management.test.js
  stdio:
    - 0
    - pipe
    - 2
  cwd: D:\a\thread-stream\thread-stream
  exitCode: 1

I believe the value should be 60 seconds ? For reference, Pino is using 480.

[ggrossetie]

I can't find why tests are failing from time to time 😞

[mcollina]

They are flaky :(

[jsumners]

Looks good to me.

[jsumners]

Cue the future issue where people transpiling everything will complain about needing the package.json....

[mcollina]

please adopt the same trick we use in pino and fastify for keeping the version up to date without having to load package.json.

[ggrossetie]

They are flaky :(

Yes but I'm trying to understand why. It works fine when I run each test individually using node. We are also using tap sequentially using jobs: 1 so I guess it's not a race condition. In theory, we should not share anything between tests so I'm a bit clueless. Maybe we should make sure that everything is closed/terminated using t.afterEach.
Anyway, I will investigate a bit further 😄

please adopt the same trick we use in pino and fastify for keeping the version up to date without having to load package.json.

• exported (static) version: https://github.com/pinojs/pino/blob/master/lib/meta.js
• script to update the version: https://github.com/pinojs/pino/blob/master/build/sync-version.js

But I don't see when sync-version.js is called? I would have call this script in a prepublishOnly task no?

[coveralls]

Pull Request Test Coverage Report for Build 2999159363

• 0 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 46.019%

---

Totals
Change from base Build 2991804988:	0%
Covered Lines:	182
Relevant Lines:	356

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 2999242094

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 46.019%

---

Totals
Change from base Build 2994447695:	0%
Covered Lines:	182
Relevant Lines:	356

---

💛 - Coveralls

[ggrossetie]

The plot thickens!
I was able to fix the flakiness with 1e3f804

Apparently, adding require('why-is-node-running') prevents an infinite loop from occurring... 🤔

[mcollina]

lgtm

[jsumners]

I think a separate issue should be opened and worked on to investigate why this is "fixing" the test suite.

[mcollina]

I think there is some weird interaction between tap own use of async_hooks, worker_threads and code coverage.

[jsumners]

It's affecting the Pino tests as well. We get flakes over there all the time.