Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix tests in Windows environment #751

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jessicatarra
Copy link
Contributor

@jessicatarra jessicatarra commented Aug 19, 2024

Description

Hello Everyone 👋. Some tests were failing when running in Windows environments, so this PR addresses this issue and offers a fix.

Fixes: #757

Type of Change

  • feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
  • ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 refactor -- Code refactor
  • ci -- Build configuration change
  • 📝 docs -- Documentation
  • 🗑️ chore -- Chore

Copy link

docs-page bot commented Aug 19, 2024

To view this pull requests documentation preview, visit the following URL:

melos.invertase.dev/~751

Documentation is deployed and generated using docs.page.

@jessicatarra jessicatarra force-pushed the test/fix-windows-tests branch from de85b1c to 8d93aea Compare August 19, 2024 18:13
@jessicatarra
Copy link
Contributor Author

@spydon even though this fix addresses the issue, it's a very hacky solution and I'm still testing in order to figure out the root cause of the problem

@jessicatarra jessicatarra force-pushed the test/fix-windows-tests branch 2 times, most recently from 2e9174b to 0e1664c Compare August 19, 2024 21:39
Comment on lines +117 to +120
# TODO: In theory, `melos test --no-select` under the hood is the same
# as the following command. For some reason, using this one directly
# provides the actual exit code of the process, avoiding
# always returning 'SUCCESS' even when tests are failing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like something we should file a bug for

Copy link
Contributor Author

@jessicatarra jessicatarra Aug 20, 2024

Choose a reason for hiding this comment

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

Yes, I agree

@@ -46,6 +48,7 @@ void main() {

logger = TestLogger();
final config = await MelosWorkspaceConfig.fromWorkspaceRoot(workspaceDir);
expectedExitCode = currentPlatform.isWindows ? 0 : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the expected exit code 0 for windows? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, that's what I'm trying to figure out. I'm not really sure what's going on. It's the same issue in the run_test and even the windows_test step in the pipeline have this problem, exit code it's 1 but is 0 in windows environments.

@jessicatarra jessicatarra force-pushed the test/fix-windows-tests branch 7 times, most recently from 01898ef to 7eaeef0 Compare August 20, 2024 02:38
@jessicatarra jessicatarra force-pushed the test/fix-windows-tests branch from 7eaeef0 to d6b707f Compare August 20, 2024 03:11
@jessicatarra
Copy link
Contributor Author

@spydon, It seems like the exit code when running in Windows environments is not reliable, and this causes issues when using some Melos commands.

@spydon
Copy link
Collaborator

spydon commented Aug 20, 2024

@spydon, It seems like the exit code when running in Windows environments is not reliable, and this causes issues when using some Melos commands.

Hmm, there must be some underlying issue for that...

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.

fix: Tests Failing in Windows Environment
2 participants