Executor: Remove duplicate entry with dry-run argument

[mmacchia]

Pull Request Check List

Resolves: #3097

• Added tests for changed code. I adapted the test in the tests/console/commands/plugin/test_remove.py file to reflect the bug fix.
• Updated documentation for changed code.

Apart from running the ci linting steps and pytest locally for both python 3.6 and 3.8, I also tested the code by running

docker run --rm -i --entrypoint bash python:3.8 <<EOF
set -e
python -m pip install -q git+https://github.com/mmacchia/poetry.git@issue/3097
python -m poetry new foobar
pushd foobar
sed -i /pytest/d pyproject.toml
python -m poetry add --dry-run pycowsay
python -m poetry run pip list
EOF

The corresponding output is:

Created package foobar in foobar
/foobar /
Creating virtualenv foobar-lWDpn5M1-py3.8 in /root/.cache/pypoetry/virtualenvs
Using version ^0.0.0.1 for pycowsay

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 1 install, 0 updates, 0 removals

  • Installing pycowsay (0.0.0.1)
Package    Version
---------- -------
pip        21.2.4
setuptools 58.1.0
wheel      0.37.0

As expected, the command poetry add --dry-run pycowsay yielded a single output ( • Installing pycowsay (0.0.0.1)) and did not add the package to the project (see the output of poetry run pip list).

The change in the PR solves the duplication issue because the creation of the console output when not self._enabled is taken care of by the boolean output of theself._should_write_operation(operation) function.

Please feel free to get in touch if I can help adding more test cases for the --dry-run argument or the _enabled flag and to add any feedback. I would be happy to improve/clarify the code in this PR.

[mmacchia]

Moreover, I tested the difference of the new and old code on the pyproject.toml file presented in the original issue, link.

Please compare the output of

docker run --rm -i --entrypoint bash python:3.8 <<EOF
set -e
python -m pip install -q git+https://github.com/mmacchia/poetry.git@issue/3097
python -m poetry new foobar
pushd foobar
curl https://gist.githubusercontent.com/MarcoGlauser/804cbf48001f7a7d1d94d6da9bc88071/raw/87d3273a0e7b86d03023b2dfcc85567b80161d3d/pyproject.toml > pyproject.toml
python -m poetry update --dry-run
python -m poetry run pip list
EOF

and

docker run --rm -i --entrypoint bash python:3.8 <<EOF
set -e
python -m pip install -q git+https://github.com/python-poetry/poetry.git@master
python -m poetry new foobar
pushd foobar
curl https://gist.githubusercontent.com/MarcoGlauser/804cbf48001f7a7d1d94d6da9bc88071/raw/87d3273a0e7b86d03023b2dfcc85567b80161d3d/pyproject.toml > pyproject.toml
python -m poetry update --dry-run
python -m poetry run pip list
EOF

[mmacchia]

One last question: as I said above, I successfully run the commands poetry run pytest tests and poetry run pre-commit run --all-files both in a python v3.6 and v3.8 environment. However it seems that the tox CI pipeline failed on v3.6 with a ('Connection aborted.', ConnectionResetError(54, 'Connection reset by peer')) error. Is there any way to address this? Is it possible to re-trigger the build on python 3.6? It looks like an SSL issue in the instance provisioned for CI.

[mmacchia]

I finally rebased my branch and the tests now passed in the CI instance, thank you for updating them! Please feel free to have a look at the suggested changes now.

[neersighted]

Hi @mmacchia -- could you please rebase this so it can be merged into master?

[mmacchia]

Hi @mmacchia -- could you please rebase this so it can be merged into master?

Hello @neersighted ! Thank you for picking this up. I managed to rebase after some troubleshooting.
Please feel free to review the changes now.

Would it be possible to squash the commits when you believe this PR is ready to be merged?

[branchvincent]

thanks!

[github-actions]

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.