Skip to content

Conversation

gribnoysup
Copy link
Collaborator

@gribnoysup gribnoysup commented Jun 20, 2021

packages/compass postinstall script currently rebuilds application dependencies with electron-rebuild which causes them to be not compatible with node version that some test suites are using, this causes flakiness when running tests on GitHub CI (see #2277 and some other PRs where tests were failing with the similar error)

This is safe to remove and will not affect Compass application packaging because when hadron-build packages the application it will rebuild native modules using electron-rebuild after reinstalling application dependencies (see packages/hadron-build/commands/release.js#L366-L394)

For some additional context the reason it's only causing issues some of the time is that in some test suites we rebuild those native modules to work in electron and then back to work in node after tests are finished, so if the stars are aligned correctly and the first test suite rebuilds native modules back to the node runtime, everything works fine.

Test runs in this PR will not really show that it fixes the issue as you need this specific order of tests running to make the issue appear, but I tried it locally against #2277 and all tests were passing with the fix applied on top of the branch

@gribnoysup
Copy link
Collaborator Author

gribnoysup commented Jun 20, 2021

Build workflow failures are not caused by anything changed in this patch and related to the issues outlined in #2279

Copy link
Collaborator

@mcasimir mcasimir left a comment

Choose a reason for hiding this comment

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

Nice!

@gribnoysup gribnoysup merged commit c232935 into master Jun 21, 2021
@gribnoysup gribnoysup deleted the no-postinstall-for-compass branch June 21, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants