Page MenuHomePhabricator

MetaTestCaseClass does check if properties like "dry", "cached" are present in subclasses
Closed, ResolvedPublic

Description

Unless a test is explicitly defined with "dry = True" it won't be in “dry” mode. For example https://gerrit.wikimedia.org/r/170577 couldn't work when dry, as it then needs to be set source explicitly:

ERROR: test_valid (tests.link_tests.TestLink)

Traceback (most recent call last):

File "/home/xzise/Programms/core/tests/link_tests.py", line 33, in test_valid
  self.assertEqual(Link('Sandbox').title, 'Sandbox')
File "/home/xzise/Programms/core/pywikibot/page.py", line 3972, in __init__
  self._source = source or pywikibot.Site()
File "/home/xzise/Programms/core/pywikibot/__init__.py", line 573, in Site
  _sites[key] = interface(code=code, fam=fam, user=user, sysop=sysop)
File "/home/xzise/Programms/core/tests/aspects.py", line 266, in __init__
  % (fam, code))

SiteDefinitionError: Loading site test:test during dry test not permitted

As nose executes dry test it shouldn't have passed jenkins. Because the test worked on Travis ( https://travis-ci.org/wikimedia/pywikibot-core/jobs/39753290#L549 ) so it's probably not related to jenkins, tox or nose.

Querying 'self.dry' inside a test returned True so it's not like it doesn't set anything.


Version: core-(2.0)
Severity: normal

Details

Reference
bz72885

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:58 AM
bzimport added a project: Pywikibot-tests.
bzimport set Reference to bz72885.
bzimport added a subscriber: Unknown Object (????).

Okay a quick test reveals that the metaclass just checks the variables directly set in the class and not in the subclasses.

In theory the subclass would've to check the subclassed classes and so on and read dry from there. This is for example also when I'm using WikidataTestCase instead of DefaultDrySiteTestCase.

gerritadmin wrote:

Change 170592 had a related patch set uploaded by XZise:
[FIX] Tests: Actually do dry site tests

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

gerritadmin wrote:

Change 170592 merged by jenkins-bot:
[FIX] Tests: Actually do dry site tests

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

Okay 170592 should've fixed that bug. All attributes required in the dct of the metaclass need to be gathered from the baseclasses which was already done for all except dry which caused two effects which happen due to the condition in tests.aspects:

https://git.wikimedia.org/blob/pywikibot%2Fcore.git/f3bf9705745b00933345e6cc8ad5b2d572f45561/tests%2Faspects.py#L522

1.) nose didn't execute all dry tests which didn't added 'dry = True' explicitly because when the condition is False (which was the case in this conditions) it always set 'net' to True which aren't executed by nose.
2.) There was no error in Travis because this condition is False so the 'DisconnectedSiteMixin' wasn't added which prevented scripts from creating an actual APISite object. That class actually is never added (as of this comment) manually.

So luckily through the first effect it was avoided that any non-dry tests where executed in nose via the second effect.

For the future the role is simple: If an attribute is required in new of the metaclass if must be also read from the baseclasses.