[CDX-180] Add volta config to repo #394
Conversation
esezen
changed the title
jjl014
left a comment
jjl014
left a comment
There was a problem hiding this comment.
Apologies for the delay here. The changes look good to me, but I'm not sure if the changes to the package-lock.json should be here though. 🤔
It also looks like the version of node we're pinning to is causing some issues with the tests.
package.json
Outdated
| "directory": "src/types/tests" | ||
| }, | ||
| "volta": { | ||
| "node": "22.18.0", |
There was a problem hiding this comment.
Should we be pinning the node version to 18.13.0 here? It looks like a lot of the tests are failing when using 22.18.0.
There was a problem hiding this comment.
Sorry for not including the context here. I messaged Evan on Slack but would have been better to add the details here.
This PR is a copy of Evan's PR here. I merged it then realized the test issue you mentioned. I reverted it and reopened this one. @evanyan13 is going to investigate why tests are failing and if we can figure it out, we are going to keep 22. If not, we can pin to 18. The strange things is, if you run tests module by module, they all pass. Only when you run them all together they fail 🤔
There was a problem hiding this comment.
Sounds good, and that is very odd.. 🤔 Thank you guys for looking into it!
There was a problem hiding this comment.
Hi @jjl014 @esezen
Apologies for the late reply on this.
Took me a while to investigate the issue but I can't seem to figure out why this is happening as the test cases that fail every time is pretty random. But all of them seems to be timeout error:
Error: Timeout of 5000ms exceeded.
I have a suspicion this might be due to how different version of Node handles the Timer module but I would need more time to test and validate this.
Considering that this story has been left hanging for quite a while now, can we just pin 18 for now and backlog this?
There was a problem hiding this comment.
@evanyan13 Should the volta config in package.json be pinned to v18 so tests pass locally as well?
|
I approve but can't approve because I am the owner of the PR |
No description provided.