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

lookup: drop mime and jest from CITGM #1063

Closed
wants to merge 2 commits into from
Closed

Conversation

RafaelGSS
Copy link
Member

As discussed in the last Release WG meeting, we'll remove the offending modules and according to #1060, those modules are failing often unrelated to machine errors.

cc: @nodejs/releasers

@marco-ippolito
Copy link
Member

Should we ping the maintainers first?

@RafaelGSS
Copy link
Member Author

Sure, @SimenB @broofa

@SimenB
Copy link
Member

SimenB commented Jul 5, 2024

👋

The linked gist with logs in #1060 (comment) has no data - did it timeout? If yes, can we increase the timeout? If we cannot increase the timeout I'd like for us to skip some tests rather than removing Jest entirely (e.g. watch and transform tests take almost a minute, even on my m2 max)

@broofa
Copy link

broofa commented Jul 5, 2024

I'm unable to view the mime test report linked at the bottom of #1060 (comment). (I get this same error for everything on Jenkins, though... so maybe an issue with my account permissions?)

@RafaelGSS
Copy link
Member Author

It was not a timeout and yeah, I think you won't be able to see the Jenkins UI since we are in lockdown due to the security release.

I can re-run it after the security release, but both modules were failing due to "MODULE_NOT_FOUND". Jest: https://gist.github.com/RafaelGSS/57c21139d9f27e855217d5f1e59ff511.

@broofa
Copy link

broofa commented Jul 5, 2024

FWIW, that gist appears to be specific to jest. It refers to the isolated-vm module, which isn't a part of mime's source or dependencies.

So... I guess ping me when the lockdown is lifted? (Sorry, I'm not familiar enough with the CITGM project to know how lockdowns work or how long they last.)

@targos
Copy link
Member

targos commented Jul 5, 2024

@broofa
Copy link

broofa commented Jul 6, 2024

Okay, I've pushed mime@4.0.4 which should fix this issue. (npm test script wasn't compiling TS files)

Hit me up if the problem doesn't go away.

@targos
Copy link
Member

targos commented Jul 6, 2024

Thanks! CI for mime: https://github.com/nodejs/citgm/actions/runs/9818510101

@targos
Copy link
Member

targos commented Jul 6, 2024

@RafaelGSS mime is passing again (we already skip it on win32)

@SimenB
Copy link
Member

SimenB commented Jul 7, 2024

The Jest one is a red herring - it's just a logged error not an actual failure. https://github.com/jestjs/jest/blob/9c9ce8ad80c6719c3378dd0058c173830621a3ff/e2e/__tests__/nativeEsm.test.ts#L31

I can probably avoid logging it, tho.

EDIT: jestjs/jest#15166

@RafaelGSS
Copy link
Member Author

Thank you folks!

@RafaelGSS RafaelGSS closed this Jul 8, 2024
@ljharb ljharb deleted the drop-mime-and-jest branch July 8, 2024 22:05
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

Successfully merging this pull request may close these issues.

5 participants