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

Emit export errors to stderr to prevent invalid output #4110

Merged
merged 3 commits into from
Sep 3, 2021

Conversation

colindean
Copy link
Contributor

An error in export could emit error messages to stdout, causing the requirements.txt output to stdout to be invalid.

Pull Request Check List

Resolves: #4109

  • Added tests for changed code.
  • Updated documentation for changed code.

N.b. I made this change in GitHub so I did not run tests or add tests to ensure the bad behavior doesn't happen.

An error in export could emit error messages to stdout, causing the requirements.txt output to stdout to be invalid.

Fixes python-poetry#4109
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Thanks a lot 👍

Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

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

Some tests are failing in tests/console/commands/test_export.py because the output is checked in stdout.

So you have to change tester.io.fetch_output() to tester.io.fetch_error() where needed.

@sonarcloud
Copy link

sonarcloud bot commented Jun 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@colindean
Copy link
Contributor Author

Fixed! I noted that when I ran tests with poetry run pytest tests/, they all pass, but when I run with make test, nearly all tests fail. This happens on master branch, too.

@jleclanche
Copy link
Contributor

@finswimmer This is a simple one, good to merge? Your requested change has been addressed, the remaining test failure looks unrelated ("connection reset by peer")

@finswimmer
Copy link
Member

@colindean: Could you please rebase your branch, to force the checks to run again?

@colindean
Copy link
Contributor Author

Checks are running now!

@finswimmer finswimmer merged commit c320955 into python-poetry:master Sep 3, 2021
1nF0rmed pushed a commit to 1nF0rmed/poetry that referenced this pull request Nov 15, 2021
…#4110)

* Emit export errors to stderr to prevent invalid output

An error in export could emit error messages to stdout, causing the requirements.txt output to stdout to be invalid.

Fixes python-poetry#4109

* Looks at stderr for export tests
edvardm pushed a commit to edvardm/poetry that referenced this pull request Nov 24, 2021
…#4110)

* Emit export errors to stderr to prevent invalid output

An error in export could emit error messages to stdout, causing the requirements.txt output to stdout to be invalid.

Fixes python-poetry#4109

* Looks at stderr for export tests
@finswimmer finswimmer mentioned this pull request Mar 6, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exported requirements.txt contains spurious error message, making it unparseable
3 participants