Skip to content

Conversation

@caalador
Copy link
Contributor

@caalador caalador commented Dec 1, 2025

Fix relative path for npm-cli
Validate found alternative node
Remove validate npm as the faulty
version 6.0 should never be installed.

Fix relative path for npm-cli
Validate found alternative node
Remove validate npm as the faulty
version  6.0 should never be installed.
@github-actions github-actions bot added the +0.0.1 label Dec 1, 2025
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Test Results

1 300 files  ± 0  1 300 suites  ±0   1h 15m 49s ⏱️ - 1m 37s
9 099 tests ± 0  9 031 ✅ ± 0  68 💤 ±0  0 ❌ ±0 
9 554 runs  +20  9 478 ✅ +20  76 💤 ±0  0 ❌ ±0 

Results for commit bebf14f. ± Comparison against base commit a3c4d35.

♻️ This comment has been updated with latest results.

@mshabarov mshabarov requested a review from mcollovati December 1, 2025 12:33
Comment on lines +320 to +323
// Running npm using node and npm-cli.js script by default
assertEquals(5, tools.getNpmExecutable().size());
assertThat(tools.getNpmExecutable().get(0), containsString("node"));
assertThat(tools.getNpmExecutable().get(1), containsString("npm"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: wasn't the goal of the test to verify that global node and npm executables are used instead of stuff installed by Flow?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like some tests fail on Bender with these changes

TaskRunNpmInstallTest.runNpmInstall_vaadinHomeNodeIsAFolder_throws
java.lang.AssertionError: Expected test to throw exception with message a string containing "it's either not a file or not a 'node' executable."

TaskRunPnpmInstallTest.runNpmInstall_vaadinHomeNodeIsAFolder_throws
java.lang.AssertionError: Expected test to throw exception with message a string containing "it's either not a file or not a 'node' executable."

FrontendToolsTest.homeNodeIsNotForced_useGlobalNode
java.lang.AssertionError: 
Expected: not a string containing "/tmp/junit3521848545615054060/junit3043236432058057772"
     but: was "/tmp/junit3521848545615054060/junit3043236432058057772/node-v24.10.0/bin/node"
java.lang.AssertionError:
Expected: not a string containing "/tmp/junit3521848545615054060/junit3043236432058057772"
     but: was "/tmp/junit3521848545615054060/junit3043236432058057772/node-v24.10.0/bin/node"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Must have mixed the methods as the one before is force alternative.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed throws tests as we install on problem and do not throw anymore.
Reverted the global folder check.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there's still something not working properly. On Bender there's global node v24.11.0 installed, but Flow seems to install v24.10.0 anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. So problematic as there is no clear indication what is happening as all tests pass locally. Will try with some log out for the global test.
Also the reinstallations are not happening it seems for the directory on bender.

Fix throws test to expect
reinstall of node instead
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
and they could clash on the CI as a system property is set.
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
move main method from test class
to own executor class
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@caalador caalador force-pushed the fix/slow-node-tests branch from b493bf8 to 7a54ca0 Compare December 3, 2025 09:16
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@caalador caalador force-pushed the fix/slow-node-tests branch from 7a54ca0 to 69989fb Compare December 3, 2025 09:21
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@caalador caalador force-pushed the fix/slow-node-tests branch 2 times, most recently from 86ea9fd to 8067aff Compare December 3, 2025 09:28
@caalador caalador force-pushed the fix/slow-node-tests branch from 8067aff to 19d40f4 Compare December 3, 2025 09:33
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
@vaadin vaadin deleted a comment from github-actions bot Dec 3, 2025
and the other one doesn't
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 3, 2025

@caalador caalador merged commit 1af987a into main Dec 3, 2025
34 checks passed
@caalador caalador deleted the fix/slow-node-tests branch December 3, 2025 11:12
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 25.0.0-beta9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants