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.

Sign up for GitHub

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

Jump to bottom

Mark tests using remote svn and hg as xfail #8790

Merged
merged 1 commit into from
Aug 24, 2020
Merged

Mark tests using remote svn and hg as xfail #8790

merged 1 commit into from
Aug 24, 2020

Conversation

McSinyx
Copy link
Contributor

@McSinyx McSinyx commented Aug 20, 2020 •
edited
Loading

colorstudy.com has been down for a few days now. TBH I'm not sure how long will GitHub keep supporting SVN checkouts, but for the moment I think it is OK.

Edit: This doesn't seem to work. Is it OK to mark the 4 subversion tests as xfail?

@McSinyx McSinyx force-pushed the fix-svn-tests branch 2 times, most recently from 7611e57 to 51d77e6 Compare August 20, 2020 10:34
@pfmoore
Copy link
Member

pfmoore commented Aug 20, 2020

Maybe we should just drop these tests? I'm not sure that svn support is that important for open source any more, and if corporate users want us to keep the tests, maybe they could donate a public-facing svn instance we could test against?

(Note: I'm not saying we drop support for subversion, that's a different question)

@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 20, 2020

I will modify this PR to drop these tests then. svn checkout seems to work for 51d77e6 but I don't understand the other error:

svnadmin: E140001: Malformed dumpfile header '<!DOCTYPE html>'

@eamanu
Copy link
Contributor

eamanu commented Aug 20, 2020

@McSinyx maybe you need update the title of this PR.

@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 20, 2020

Thanks for the heads up! In other news BitBucket stops supporting Mercurial )-:

@McSinyx McSinyx changed the title Use GitHub for SVN tests Use git in place of svn for tests Aug 20, 2020
pfmoore
pfmoore reviewed Aug 20, 2020
View reviewed changes
tests/functional/test_install_user.py Outdated
@@ -58,7 +56,8 @@ def test_install_subversion_usersite_editable_with_distribute(
'install', '--user', '-e',
'{checkout}#egg=initools'.format(
checkout=local_checkout(
'svn+http://svn.colorstudy.com/INITools', tmpdir)
'git+https://github.com/ianb/initools', tmpdir,
)
Copy link
Member

Choose a reason for hiding this comment

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

Seems wrong to use git for a test named test_install_subversion_usersite_editable_with_distribute. Are these not tests specifically designed to test subversion features?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed the test to test_install_git_usersite_editable_with_distribute now. It seems to be the only test doing editable install for user and subversion was used for historical reasons.

@pfmoore
Copy link
Member

pfmoore commented Aug 20, 2020

In other news BitBucket stops supporting Mercurial )-:

They've been warning that was coming for months. In fact, I thought it had happened ages ago.

eamanu
eamanu reviewed Aug 20, 2020
View reviewed changes
Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

Maybe you could fix here test_install_global_option_using_editable? that is also fail

@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 21, 2020 •
edited
Loading

The BitBucket repo doesn't drop hg support (yet), but now requires authentication. I suppose for now it's OK to replace it with another one even it's git instead.

Edit: BitBucket really dropped hg support.

@McSinyx McSinyx requested a review from pfmoore August 21, 2020 14:57
@pfmoore
Copy link
Member

pfmoore commented Aug 21, 2020

I honestly have no idea if it's OK to switch from svn/hg to git for these tests. So I'm going to defer to other @pypa/pip-committers on whether to merge this.

@pfmoore pfmoore removed their request for review August 21, 2020 15:05
@uranusjr
Copy link
Member

uranusjr commented Aug 21, 2020 •
edited
Loading

Instead of switching away from SVN, maybe we should initialise an SVN repository locally, and access it with file:// in tests instead. It is not difficult at all (just a couple of svnadmin calls), and there’s already a test helper for that.

@sbidoul
Copy link
Member

sbidoul commented Aug 22, 2020

initialise an SVN repository locally, and access it with file://

Which reminds me that we'd be blocked by this packaging issue: pypa/packaging#264

@uranusjr
Copy link
Member

This is fine

@eamanu
Copy link
Contributor

eamanu commented Aug 22, 2020

IMHO, we should mark those tests as xfail because we are blocking all new PR. Also master will fail?
After that, we can continue working on the tests

@uranusjr
Copy link
Member

I agree, that feels like the best we can do before packaging is updated to allow svn+file://.

@McSinyx McSinyx force-pushed the fix-svn-tests branch 2 times, most recently from 0c764ba to d429b39 Compare August 23, 2020 13:40
@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 23, 2020 •
edited
Loading

I marked all tests using svn.colorstudy.com as xfailed now. I just want to note that though I don't understand what the svn helper is doing:

pip/tests/lib/local_repos.py

Lines 17 to 37 in e1d489f

def _create_svn_initools_repo(initools_dir):
"""
Create the SVN INITools repo.
"""
directory = os.path.dirname(initools_dir)
subprocess.check_call('svnadmin create INITools'.split(), cwd=directory)
filename, _ = urllib_request.urlretrieve(
'http://bitbucket.org/hltbra/pip-initools-dump/raw/8b55c908a320/'
'INITools_modified.dump'
)
devnull = open(os.devnull, 'w')
dump = open(filename)
subprocess.check_call(
['svnadmin', 'load', initools_dir],
stdin=dump,
stdout=devnull,
)
dump.close()
devnull.close()
os.remove(filename)

It seems that a regular svn checkout, e.g. from https://github.com/ianb/INITools, won't just work, and that the dump file is no longer accessible due to BitBucket support for the repo (which was managed by hg) is now retired.

@McSinyx
Mark tests using remote svn and hg as xfail
c84ef7a 
The source repositories for testing is no longer available.
@McSinyx McSinyx changed the title Use git in place of svn for tests Mark tests using remote svn and hg as xfail Aug 24, 2020
@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 24, 2020

Gently pokes @uranusjr and @sbidoul for a quick merge.

@uranusjr
Copy link
Member

I think we’ll need an admin to do this by hand since both this and #8791 fail some required checks and can’t be merged automatically.

@pradyunsg
Copy link
Member

I think we’ll need an admin to do this by hand

Not just by hand, one of us needs to go and disable certain protections to be able to make these merges. :)

@pradyunsg pradyunsg merged commit 1c30312 into pypa:master Aug 24, 2020
@McSinyx McSinyx deleted the fix-svn-tests branch August 24, 2020 07:50
@McSinyx
Copy link
Contributor Author

McSinyx commented Aug 24, 2020

Thanks. I don't know who are admins and at this point I'm too afraid to ask.

@McSinyx
Copy link
Contributor Author

McSinyx commented Oct 12, 2020

I've just migrated a repo to https://hg.sr.ht/~cnx/lazip, to be used in the future if we ever want to test installing from mercurial again.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eamanu eamanu eamanu left review comments

@pfmoore pfmoore pfmoore left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@McSinyx @pfmoore @eamanu @uranusjr @sbidoul @pradyunsg