test_load_from_pageids_pipe_separated is failing on Travis
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Magul
	Dec 18 2016, 2:02 PM

Description

________ TestLoadPagesFromPageids.test_load_from_pageids_pipe_separated ________
self = <tests.site_tests.TestLoadPagesFromPageids testMethod=test_load_from_pageids_pipe_separated>
    def test_load_from_pageids_pipe_separated(self):
        """Test loading from comma-separated pageids."""
        pageids = '|'.join(str(page._pageid) for page in self.links)
        gen = self.site.load_pages_from_pageids(pageids)
>       for count, page in enumerate(gen, start=1):
tests/site_tests.py:2568: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pywikibot/site.py:3148: in load_pages_from_pageids
    for sublist in itergroup(filter_unique(gen), groupsize):
pywikibot/tools/__init__.py:637: in itergroup
    for item in iterable:
pywikibot/tools/__init__.py:876: in filter_unique
    for item in iterable:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.0 = <list_iterator object at 0x7f12de6724a8>
>   gen = (str(int(p)) for p in pageids if int(p) > 0)
E   ValueError: invalid literal for int() with base 10: ''
pywikibot/site.py:3139: ValueError

See: https://travis-ci.org/wikimedia/pywikibot-core/jobs/184925687#L1902

Details

	Subject	Repo	Branch	Lines +/-
	Make pageids validation's error more descriptive	pywikibot/core	master	+9 -1
	load_pages_from_pageids: do not fail on empty string	pywikibot/core	master	+7 -0

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Magul	T151145 Make Travis (and Appveyor) pass again
		Resolved		Magul	T153592 test_load_from_pageids_pipe_separated is failing on Travis

Event Timeline

Magul created this task.Dec 18 2016, 2:02 PM

Change 328044 had a related patch set uploaded (by Magul):
Change pageids validation to more error resilient

https://gerrit.wikimedia.org/r/328044

gerritbot added a project: Patch-For-Review.Dec 18 2016, 2:37 PM

Change 337348 had a related patch set uploaded (by Dalba):
site.py: load_pages_from_pageids do not fail on empty string

https://gerrit.wikimedia.org/r/337348

Xqt closed this task as Resolved.Mar 4 2017, 8:40 PM

Change 337348 merged by jenkins-bot:
[pywikibot/core] load_pages_from_pageids: do not fail on empty string

https://gerrit.wikimedia.org/r/337348

In T153592#2885061, @gerritbot wrote:

Change 328044 had a related patch set uploaded (by Magul):
Change pageids validation to more error resilient

https://gerrit.wikimedia.org/r/328044

@Dalba @Xqt @Magul: Should this patch be abandoned with the merging of https://gerrit.wikimedia.org/r/337348?

In T153592#3073899, @Lokal_Profil wrote:

@Dalba @Xqt @Magul: Should this patch be abandoned with the merging of https://gerrit.wikimedia.org/r/337348?

Apparently that patch is pursuing another goal: "Make pageids validation's error more descriptive". But personally, I don't find think it's worth the added complexity and it's not common for users to pass something other than valid pageids to load_pages_from_pageids. In the rare case that they do I find the ValueError: invalid literal for int() with base 10: '' descriptive enough.

In T153592#3237572, @Dalba wrote:

In T153592#3073899, @Lokal_Profil wrote:

@Dalba @Xqt @Magul: Should this patch be abandoned with the merging of https://gerrit.wikimedia.org/r/337348?

Apparently that patch is pursuing another goal: "Make pageids validation's error more descriptive". But personally, I don't find think it's worth the added complexity and it's not common for users to pass something other than valid pageids to load_pages_from_pageids. In the rare case that they do I find the ValueError: invalid literal for int() with base 10: '' descriptive enough.

A quick fix might be to wrap the gen = call in a try and then catch any ValueError and replace it by something more descriptive?

In T153592#3237740, @Lokal_Profil wrote:

A quick fix might be to wrap the gen = call in a try and then catch any ValueError and replace it by something more descriptive?

On the other hand then one would have to parse the original error message to get the offending string out. You are right that it doesn't seem worth it. I would abandon that patch and mark this task as resolved.

Change 328044 abandoned by Xqt:
[pywikibot/core@master] Make pageids validation's error more descriptive

Reason:
seems T153592 is solved already