MigrationTest::migratePnpmPassesHappyPath is flaky in Flow 2.5, 2.6 Java 11 Nightly

[mshabarov]

https://bender.vaadin.com/project.html?projectId=Flow_Flow26_Nightly&testNameId=7384110105454118410&tab=testDetails

https://bender.vaadin.com/viewLog.html?buildId=220939&tab=buildResultsDiv&buildTypeId=Flow_Flow26_Nightly_Java11Nightly#testNameId7384110105454118410

java.lang.AssertionError: Unexpected command '[/usr/bin/npx, --yes, --quiet, pnpm, --shamefully-hoist=true, install, polymer-modulizer]' expected:<8> but was:<7>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at com.vaadin.flow.migration.MigrationTest$1.executeProcess(MigrationTest.java:228)
	at com.vaadin.flow.migration.Migration.ensureTools(Migration.java:417)
	at com.vaadin.flow.migration.Migration.migrate(Migration.java:160)
	at com.vaadin.flow.migration.MigrationTest.migratePassesHappyPath(MigrationTest.java:235)
	at com.vaadin.flow.migration.MigrationTest.migratePnpmPassesHappyPath(MigrationTest.java:169)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:54)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:273)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:383)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:344)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:125)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:417)
------- Stderr: -------
[main] INFO com.vaadin.flow.server.frontend.FrontendTools - using '/usr/bin/npx --yes --quiet pnpm' for frontend package installation
[main] INFO com.vaadin.flow.server.frontend.FrontendTools - using '/usr/bin/npx --yes --quiet pnpm' for frontend package installation

[denis-anisimov]

Link may become broken at some point.
So flaky report may not contain only the link.

It should contain exception stacktrace of the failed test and screenshot if possible.
Artifacts will be removed anyway at some point even if link is still alive.

[mshabarov]

Good comment! This test is a unit test, thus no screenshots are provided, but at least a stack-trace can be attached.

[mshabarov]

The migratePnpmPassesHappyPath test contains a condition checking which Node installation is on the machine, local or global:
if (homeNodeDir.exists() || !pnpmExecutable.get(0).contains("npx")) { ...}.

The flakiness of this test depends on which if block it passes into. Most of the time it decides to go to global installed Node block (which is correct). But sometimes it goes to 'local' block. Since the pnpm executable actually contains 'global' (7 entries instead of 8) script, it fails later on during the entries count check.

The second condition seems to be always false (global should be used), because both of the failed and passes test logs show the similar executable path:

INFO com.vaadin.flow.server.frontend.FrontendTools - using '/usr/bin/npx --yes --quiet pnpm' for frontend package installation

Thus, the first condition is a culprit of this false negative test.

Most probably, ~/.vaadin/node/ folder is created somewhere in other test and not removed afterwards, which gives a false decision that the NodeJS is installed locally.

[mshabarov]

The failure rate increased starting from 26.03.2021:

[denis-anisimov]

Migration tool is obsolete I think.
I cannot imagine that someone still uses old HtmlImport in the latest 2.6 and want to migrate to Polymer templates.
I don't know whether we need Migration tool at all ( it has limitations and doesn't work reliably) but I'm pretty sure we can just either ignore or remove migration tests:

• nobody is going to fix anything there since HtmlImports are obsolete
• nobody is going to change anything in this tool
• we have it in the previous versions and that should be enough.

I suggest do not spend time on it at all.

[mshabarov]

Thanks for valuable comment, @denis-anisimov !
True, if it's kind of legacy tool and no changes foreseen, then let's keep this test ignored.
I was just worried that this test could reveal the issue in the common node/npm/pnpm functionality, but most likely this is just an impact of another test elsewhere which installs Node into .vaadin folder or calls node installer to do that, and then doesn't make a proper cleanup.

But yes, let's not focus on that.