[npm] Randomize advisory id to avoid cache collisions across tests

[mctofu]

🤞 This fixes test failures seen when the tests are run in a particular order. Details of the advisories are being cached in the npm cache and shared across test runs. This can lead to faulty analysis of vulnerabilities in the tests because we were using the same advisory ids in each evaluation.

I didn't track down exactly where it goes wrong in the arborist code but I found that once I reproduced the failures locally with a particular test seed I could then reliably reproduce the failure on an individual test. After observing that the audit report didn't completely match up with the advisory I tried changing the advisory id and the test passed.

[deivid-rodriguez]

Wow, must've been hard to figure out 😅. So this initially reproduced by running tests with some fixed seed, and then you reduced that to running just the failing test in isolation twice, and it would fail the second time, right?

In general, I find that order dependent failures are best fixed by fixing the state leak between them. In this case, I guess clearing the npm cache of the specs that leak it?

But this may not be possible/desirable here, not sure.

In any case, it's great to finally get rid of these flakes! 🎉 🎉

[mctofu]

In general, I find that order dependent failures are best fixed by fixing the state leak between them. In this case, I guess clearing the npm cache of the specs that leak it?

Yeah, I considered that as well but was worried that it would slow down the tests since the npm cache also helps re-downloading common packages across the tests.

[mctofu]

One more note: this only affected the tests and manual/local usage of Dependabot. On github.com we use a fresh sandbox for each security update we create so the npm cache doesn't persist between vulnerabilities.

[jeffwidman]

Thanks for fixing this!