macos test failures

[eleith]

@zackschuster we are getting macos test failures on latest merge. do they pass for you locally (if you use a mac)?

[zackschuster]

yup, on both intel & apple silicon. i think the failures we're seeing are from a combination of slowness in mailparser (based on profiling) and the sheer number of targets we're running in a commodity environment, which i would unscientifically rate as being at best on par with my 2015 macbook pro. the easy fix would be removing node@10 support in v4, but really i just think we need to find a way to speed up our tests. as an example, even on an M1 Pro message.ts › streams message takes five seconds to complete.

[zackschuster]

😬 😬 😬

i'll keep looking. i hadn't noticed this before, but only macOS is this slow. linux & windows targets all finished test/message.ts in ~2s.

[zackschuster]

as of 5eea9d0 on my dev branch the tests are now completely non-blocking & complete in roughly half the time (on macOS; ubuntu & windows are slightly slower). they also still throw timeouts for the slowest tests in ci. 🤷🏻 🤷🏻 🤷🏻

as the workflow logs show, the only bottleneck is mailparser, with the usual aggravating caveat that it's on macOS only. at this point i don't think there's much (if anything) more to be done to our code to improve the situation.

[zackschuster]

i've opened an issue on the mailparser repo with my findings.

[eleith]

thanks @zackschuster, what do you think about mocking mailparser fullly and reducing the scope of the test to just internal code?

[zackschuster]

by mocking, do you mean rolling our own parser? to my mind, the value of using mailparser is that we have a trusted community module verifying that we're transmitting the data correctly; otherwise, we're essentially saying "trust us, we know what we're doing". i would be fine with replacing it with something more performant, but npm searches don't lead me to believe we even really have a second choice.

[eleith]

no. I meant only in the unit test, replacing it with a test double

[zackschuster]

yeah, that was my assumption (i don't think the library has any business shipping an email parser). the same concerns apply.

[eleith]

I just looked at the test. I see this module is used explicitly to test aspects of the library, so a double makes no sense, it's not used otherwise.

unfortunate dependency I guess.

tests are passing now, so I assume we just have flakiness? we could disable macOS if we can't get it green in the future...

[zackschuster]

I assume we just have flakiness?

yeah, specifically for the larger message tests on macOS. everything else is fine.

we could disable macOS if we can't get it green in the future...

i think at this point ci is just barely too slow on the flaky tests, so we basically need the stack or the environment to give us just about any consistent perf win & we'll be solidly green.