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

Export_failure test fails on Windows with Powershell #1061

Closed
michidk opened this issue Dec 31, 2021 · 12 comments
Closed

Export_failure test fails on Windows with Powershell #1061

michidk opened this issue Dec 31, 2021 · 12 comments

Comments

@michidk
Copy link
Contributor

michidk commented Dec 31, 2021

Regarding the export_failure unit test here: https://github.com/casey/just/blob/master/src/justfile.rs#L645

When running the code with args: ["--quiet", "--shell", "powershell.exe", "--shell-arg", "-c", "wut"], on Windows the test will fail, because no Code runtime error will occur.

I am not sure why it would fail on Linux either, maybe someone can elaborate what the purpose of this test is.

@casey
Copy link
Owner

casey commented Jan 3, 2022

Is this failing on master?

@michidk
Copy link
Contributor Author

michidk commented Jan 3, 2022

Is this failing on master?

Yes it fails on the most recent commit on master (125801e).

@casey
Copy link
Owner

casey commented Jan 3, 2022

I'm not sure I understand, this recent test run passed on windows: https://github.com/casey/just/runs/4686879325?check_suite_focus=true

@michidk
Copy link
Contributor Author

michidk commented Jan 3, 2022

I'm not sure I understand, this recent test run passed on windows: casey/just/runs/4686879325?check_suite_focus=true

But not with the following args args: ["--quiet", "--shell", "powershell.exe", "--shell-arg", "-c", "wut"]

@michidk
Copy link
Contributor Author

michidk commented Jan 3, 2022

I think the export feature is not yet compatible with PowerShell.

@casey
Copy link
Owner

casey commented Jan 3, 2022

That's okay, not all tests are expected to be compatible with PowerShell, since a lot of them use shell-specific constructs.

@casey casey closed this as completed Jan 3, 2022
@michidk
Copy link
Contributor Author

michidk commented Jan 4, 2022

Okay I was just worried that this feature wouldn't work with PowerShell at all, because it uses no sh/linux specific constructs in that test as far as I can see.

I published the unit test here: https://github.com/casey/just/pull/1066/files#diff-7f1715f83f401c5a708de4c89128b39d561b79907de38aa51ecbc421790d5a8aR547
as you can see its failing: https://github.com/casey/just/runs/4700129707?check_suite_focus=true

---- justfile::tests::export_failure_powershell stdout ----
thread 'justfile::tests::export_failure_powershell' panicked at 'Expected runtime error: ()', src\justfile.rs:547:3

@casey
Copy link
Owner

casey commented Jan 5, 2022

Okay, sweet, that's for clarifying, I didn't understand what the issue was. I'll leave #1060 open to track the issue.

@michidk
Copy link
Contributor Author

michidk commented Jan 5, 2022

I think this is a different issue than #1060. #1060 is about the exit code this one is about the export feature.

@casey
Copy link
Owner

casey commented Jan 5, 2022

Ahhhh, okay, my bad, again, totally misunderstood. I think that this is not a bug. The test is checking that baz is not exported, and that trying to evaluate $baz is an error. However, the only reason this is an error is because we pass -u to the shell by default, which makes evaluating undefined variables an error. By default, does powershell treat evaluating undefined variables as an error? If not, it probably needs to be passed an additional flag, analogous to -u.

@michidk
Copy link
Contributor Author

michidk commented Jan 15, 2022

I don't think there is such a flag in powershell

@casey
Copy link
Owner

casey commented Jan 15, 2022

I opened #1071 to track setting strict mode when windows-powershell is set.

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

No branches or pull requests

2 participants