Allow to prefetch debug info to mitigate spurious failures in the CI #3517
Conversation
| // Prefetch the debug metadata before the actual tests do start | ||
| if (LinktimeInfo.hasDebugMetadata) { | ||
| val shouldPrefetch = | ||
| sys.env.get("PREFETCH_DEBUG_INFO").exists(v => v.isEmpty() || v == "1") |
There was a problem hiding this comment.
Should env be prefixed with e.g. SN_TEST or SCALA_NATIVE_TEST ?
There was a problem hiding this comment.
I could be prefixed, but don't you think it is too verbose? When it's added in CI it's fine, but when testing locally it gives a lot of opportunities for typos. On the other hand the short version breaks naming consistency. What's your opinion on that?
There was a problem hiding this comment.
If this was private to this repo, then it would be okay, but since this is a public API feature it should follow existing convention. Verbosity is annoying but hopefully we choose the right default and most user should never need to configure it.
There was a problem hiding this comment.
I know Denys always wanted it spelled out SCALA_NATIVE_ perhaps because so many C libs use a 2 character namespacing like tf_ for Tensorflow.
There was a problem hiding this comment.
I've added the SCALANATIVE_TEST_ prefix to allign it with other env variables (eg. SCALANATIVE_GC) and added TEST to point out it's only applied when testing
There was a problem hiding this comment.
Loading debug info can take almost 1s on MacOS M1 for a simple tests (1 test in sandbox) or even 3s for our current unit tests. This can influence the assertions that expect code to finish withing some time interval, especially in multithreading builds.
Nice catch! I thought it was acceptable to take 1s to load debug info when throwing an exception, but I hadn't thought about the test cases where exceptions are expected.
Regarding the env var prefix, I personally prefer ScalaNative having some kind of prefix so that the env var name doesn't crash with others, but not a strong opinion
Loading debug info can take almost 1s on MacOS M1 for a simple tests (1 test in sandbox) or even 3s for our current unit tests. This can influence the assertions that expect code to finish withing some time interval, especially in multithreading builds.
Mitigate this issue by introducing env var
PREFETCH_DEBUG_INFO=1to ensure that debug info would be parsed before starting to execute tests