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

app-i18n/transifex-client: drop dependency to dev-python/mock #19057

Closed
wants to merge 2 commits into from

Conversation

robert7k
Copy link
Contributor

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @robert7k
Areas affected: ebuilds
Packages affected: app-i18n/transifex-client

app-i18n/transifex-client: @robert7k, @gentoo/proxy-maint

Linked bugs

Bugs linked: 765349


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Jan 13, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-01-13 23:05 UTC
Newest commit scanned: 2bb3866
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/0b1c13a38e/output.html

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

error: Could not find suitable distribution for Requirement.parse('smmap<4,>=3.0.1')
 * ERROR: app-i18n/transifex-client-0.14.2-r1::testworld failed (test phase):
 *   Tests fail with python3.7

It also has a strict version requirement...

@robert7k
Copy link
Contributor Author

error: Could not find suitable distribution for Requirement.parse('smmap<4,>=3.0.1')
 * ERROR: app-i18n/transifex-client-0.14.2-r1::testworld failed (test phase):
 *   Tests fail with python3.7

It also has a strict version requirement...

That's strange – smmap isn't even a direct dependency of transifex-client. I also couldn't reproduce this after installing smmap-4.0.0 (both with Python 3.7 and 3.9). Do you have more information on where exactly this error message comes from?

Rebased the PR on the most recent master.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-01-31 01:21 UTC
Newest commit scanned: 67b139e
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/9d1aca42e9/output.html

@ionenwks
Copy link
Contributor

ionenwks commented Jan 31, 2021

error: Could not find suitable distribution for Requirement.parse('smmap<4,>=3.0.1')

It also has a strict version requirement...

That's strange – smmap isn't even a direct dependency of transifex-client. I also couldn't reproduce this after installing smmap-4.0.0 (both with Python 3.7 and 3.9). Do you have more information on where exactly this error message comes from?

Was looking at this earlier because of https://bugs.gentoo.org/767970, this strict dependency was set nearly a year ago and 4.0.0 was released only 7 days ago. I haven't tested but like @thesamesam I question its validity.

Edit: Also it's likely coming from GitPython (which I see transifex depends on) which itself depends on gitdb, not that I looked further.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-01-31 01:36 UTC
Newest commit scanned: a6c1157
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c59618e5bf/output.html

@robert7k robert7k requested a review from juippis January 31, 2021 12:46
@ionenwks
Copy link
Contributor

https://bugs.gentoo.org/767970

Been resolved, I don't think this package need to worry about this smmap issue.

@@ -31,4 +30,5 @@ src_prepare() {
eapply_user
sed -i -e 's:test_fetch_timestamp_from_git_tree:_&:' \
tests/test_utils.py || die
sed -i ':tests_require=["mock>=3.0.5,<4.0"]:d' setup.py || die
Copy link
Member

Choose a reason for hiding this comment

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

>>> Source compiled.
>>> Test phase: app-i18n/transifex-client-0.14.2-r1
 * python3_7: running distutils-r1_run_phase python_test
python3.7 setup.py test --verbose
Warning: 'keywords' should be a list, got type 'tuple'
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
WARNING: The wheel package is not available.
/usr/bin/python3.7: No module named pip
error: Command '['/usr/bin/python3.7', '-m', 'pip', '--disable-pip-version-check', 'wheel', '--no-deps', '-w', '/var/tmp/portage/app-i18n/transifex-client-0.14.2-r1/temp/tmpfl1rd2d3', '--quiet', 'mock<4.0,>=3.0.5']' returned non-zero exit status 1.
 * ERROR: app-i18n/transifex-client-0.14.2-r1::testworld failed (test phase):
 *   Tests fail with python3.7
>: grep -i mock /var/tmp/portage/app-i18n/transifex-client-0.14.2-r1/work/transifex-client-0.14.2/setup.py 
    tests_require=["mock>=3.0.5,<4.0"],

I don't know why, but the : don't seem to work very well as delimiters. Switch to / and it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange, worked fine for me. Replaced anyway.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-02-02 18:50 UTC
Newest commit scanned: 5276763
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/094c0271f2/output.html

eapply_user
sed -i -e 's:test_fetch_timestamp_from_git_tree:_&:' \
tests/test_utils.py || die
sed -i '/tests_require=["mock>=3.0.5,<4.0"]/d' setup.py || die
Copy link
Member

@juippis juippis Feb 3, 2021

Choose a reason for hiding this comment

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

>: cd /var/tmp/portage/app-i18n/transifex-client-0.14.2-r1/work/transifex-client-0.14.2/
>: grep -i mock setup.py 
    tests_require=["mock>=3.0.5,<4.0"],
>: sed -i '/tests_require=["mock>=3.0.5,<4.0"]/d' setup.py
>: grep -i mock setup.py 
    tests_require=["mock>=3.0.5,<4.0"],
>: sed -i '/tests_require=/d' setup.py
>: grep -i mock setup.py 
>: 

Seriously what is going on? Can someone explain? Is it just me?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sed doesn't work for me either, fine if brackets are escaped.
sed -i '/tests_require=\["mock>=3.0.5,<4.0"\]/d' setup.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to escape the brackets, so I added that. Still wondering why it worked for me anyway...

Signed-off-by: Robert Siebeck <gentoo.2019@r123.de>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-02-03 18:50 UTC
Newest commit scanned: b6874a6
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/daafb3b7a0/output.html

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

So now the sed works, but the test cases are actually using mock.

======================================================================
ERROR: test_api (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_api
Traceback (most recent call last):
  File "/usr/lib/python3.7/unittest/loader.py", line 154, in loadTestsFromName
    module = __import__(module_name)
  File "/var/tmp/portage/app-i18n/transifex-client-0.14.2-r1/work/transifex-client-0.14.2/tests/test_api.py", line 4, in <module>
    from mock import MagicMock, patch
ModuleNotFoundError: No module named 'mock'


----------------------------------------------------------------------
Ran 10 tests in 0.001s

FAILED (errors=6)
Test failed: <unittest.runner.TextTestResult run=10 errors=6 failures=0>
error: Test failed: <unittest.runner.TextTestResult run=10 errors=6 failures=0>
 * ERROR: app-i18n/transifex-client-0.14.2-r1::testworld failed (test phase):
 *   Tests fail with python3.7

https://github.com/transifex/transifex-client/blob/master/tests/test_project.py#L16
https://github.com/transifex/transifex-client/blob/master/tests/test_api.py#L4

What are we doing in here? It does look like removing mock needs a bit more, ie a full patch.

EDIT: Or if possible https://dev.gentoo.org/~mgorny/python-guide/test.html#skipping-problematic-tests

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

Only now I get the point of this PR, the tests work with later version of mock, so the restriction is redundant in setup.py. We still need to BDEPEND on mock though.

Thanks for sticking it through!

@gentoo-bot gentoo-bot closed this in 378f706 Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
5 participants