appium icon indicating copy to clipboard operation
appium copied to clipboard

fix npm-related e2e test flake

Open boneskull opened this issue 2 years ago • 4 comments

The problem

Our E2E tests which install extensions from npm fail frequently due to timeouts. This may be an npm network thing, it may be a GHA infrastructure thing, it may be rate limiting--who knows. I've increased the timeout on these tests maybe 8-10 times already, and I don't want to do it anymore.

There's no guarantee that disabling the test timeouts entirely would work, either (but it might). It may be that the npm install command would never resolve. Who knows.

One way that might work to fix this is caching.

The good news: because we now have a package-lock.json under VCS, we can cache our modules in CI. In addition to the usual caching of node_modules and/or ~/.npm (which should speed up our builds anyway), we may need to add a custom cache of the modules retrieved from npm during these E2E tests. We can either a) add them to the ~/.npm cache, b) cache the entire installed extension (wherever it's installed), or c) both. A caveat with b) is that APPIUM_HOME is usually a random temp dir, so this behavior would need to be changed to be something still temporary, but static.

A second approach would be to eschew the use of the registry entirely, and just leverage npm to install a local module (e.g., @appium/fake-driver). I think this is probably OK, and I'm 99% confident it'd work. I am less confident in the previously-mentioned approach.

Aside: in fact, npm install /path/to/some/module is the same thing as using --source=local which performs an npm link. To that end, I'm fairly certain --source=local can just be an alias for --source=npm, and we shouldn't need to call npm link at all.

@jlipps Thoughts?

boneskull avatar Jul 08 '22 19:07 boneskull

If we were going to do local installs instead, I'm not sure of the best way to handle the "upgrade available" test(s)...

boneskull avatar Jul 08 '22 19:07 boneskull

I think part of the point of these tests is to actually perform an e2e test against the actual registry, so we may be able to replace some (but not all) with local tests.

I'm fairly certain --source=local can just be an alias for --source=npm

Let's try it and see if it works!

jlipps avatar Jul 12 '22 19:07 jlipps

I think part of the point of these tests is to actually perform an e2e test against the actual registry, so we may be able to replace some (but not all) with local tests.

Well... why? If we can call out to npm to have it install something, is that not good enough? At what point are we just testing that the registry is reachable?

boneskull avatar Jul 13 '22 19:07 boneskull

What if NPM changes the way that install works and introduces some difference between local and registry. I'd still like to have confidence that installing from the registry works. Maybe we can get away with just 1 test that does this. Or maybe we push this to our post deploy tests, where we are installing actual drivers rather than fake driver anyway.

jlipps avatar Jul 13 '22 19:07 jlipps