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

Let run_in_login_mode succeed even with broken local config #10622

Merged
merged 1 commit into from Oct 10, 2023

Conversation

Hofer-Julian
Copy link
Contributor

I wondered why this test failed for me.
Turns out my config file is not compatible with current main, but the error message was useless. I've added --no-config-file and improved the surrounding tests a bit while I was at it.

tests/shell/mod.rs Show resolved Hide resolved
tests/shell/mod.rs Outdated Show resolved Hide resolved
I wondered why this test failed for me.
Turns out my config file is not compatible with current main, but the error message was useless.
I've added `--no-config-file`.
@Hofer-Julian
Copy link
Contributor Author

@sholderbach I've had another look, and I am less confident now that checking the status code will do it.
I don't really have a good idea how to improve the tests for now, so I've reverted my changes and only included the original fix

@amtoine
Copy link
Member

amtoine commented Oct 8, 2023

@Hofer-Julian @sholderbach
i'm unclear about the status of this PR, is this solving the issue and ready to land? 😋

@Hofer-Julian
Copy link
Contributor Author

@Hofer-Julian @sholderbach i'm unclear about the status of this PR, is this solving the issue and ready to land? 😋

Yes, it solves the issue and IMO ready to land

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

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

Thanks! Very much good to go

@sholderbach sholderbach merged commit 8a77d1e into nushell:main Oct 10, 2023
19 checks passed
@Hofer-Julian Hofer-Julian deleted the simplify_login_test branch October 10, 2023 12:12
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
…l#10622)

I wondered why this test failed for me.
Turns out my config file is not compatible with current main, but the
error message was useless. I've added `--no-config-file`
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.

None yet

3 participants