test: fix typescript version in pack-n-play system test#345
test: fix typescript version in pack-n-play system test#345alvarowolfx merged 5 commits intogoogleapis:mainfrom
Conversation
product-auto-label
Bot
added
size: xs
product-auto-label
Bot
added
size: s
alvarowolfx
changed the title
|
Typescript 5.1 was released last week and one of the breaking changes is that Node 14+ is required (https://devblogs.microsoft.com/typescript/announcing-typescript-5-1/#breaking-changes). This is causing some issues on our repos that uses pack-n-play for installation/system tests under a Node 12 environment. The issue happens because pack-n-play tries to install the latest version of typescript for running tests ( https://github.com/google/pack-n-play/blob/master/src/pack-n-test.ts#L97 ). |
alvarowolfx
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
| try { | ||
| await packNTest(options); | ||
| } catch (err) { | ||
| console.error('TS install failed:', err); |
There was a problem hiding this comment.
I believe err here can be omitted, since it's already throwing in the next line, it should end up in stderr the same way:
| console.error('TS install failed:', err); | |
| console.error('TS install failed'); |
There was a problem hiding this comment.
Before printing the err, the output of the errors being thrown was not including the output of the pack-n-play function, so was not possible to see where the error was happening. The output was just:
Error: Process 227 exited with code 1.
at ChildProcess.<anonymous> (node_modules/pack-n-play/build/src/utils.js:48:22)
-> /workspace/node_modules/pack-n-play/src/utils.ts:62:11
at maybeClose (internal/child_process.js:1022:16)
at Process.ChildProcess._handle.onexit (internal/child_process.js:287:5)
There was a problem hiding this comment.
oh I see! it looks like pack-n-play is swallowing these errors then 🤔
alright, in this case it sounds reasonable to me to have it printed here for sure! personally I'd put in the next line just to make it more readable, e.g:
| console.error('TS install failed:', err); | |
| console.error('TS install failed:\n', err); |
but that's def an optional nit
alvarowolfx
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
2 participants
Pin linkinator to v4.1.2 to avoid Node 16+ reqlinkinator was not the issue, was actually caused by latest release of Typescript not being compatible with Node 12 ( min requirement 14+ ).