Xqt added a comment.

  In T223030#5175098 <https://phabricator.wikimedia.org/T223030#5175098>, 
@Dvorapa wrote:
  
  > Could you explain the reason behind the changes a little bit more?
  >
  > I understand that the behavior is different between Python 2 and 3, but why 
should we remove it now?
  >
  > Also in the script_tests.py 
<https://gerrit.wikimedia.org/r/#/c/pywikibot/core/+/509631/> you changed 
allowed_failure to skip, which is correct in the script_tests, but you write 
the exact opposite ("should be changed to expectedFailure") in the description 
of the task abovel, so I'm a bit confused.
  
  
  I see several points that `allowed_failure` and it conditional 
`allowed_failure_if` should not be used at all; both indicate an expected 
failure (handled by expectedFailure) but it is handled in a different way:
  
  Default behavior without overwriting result in aspects (see patches 
"expected"/"unexpected"
  
  | **Decorator**   | Exception      | **Testing** | **Behavior**       | 
**Test result PY 2.7 ** | ** Test result PY >= 3.4** |
  | expectedFailure | AssertionError | yes         | expected failure   | OK    
                  | OK                         |
  | expectedFailure | any Exception  | yes         | error              | 
FAILED                  | FAILED                     |
  | expectedFailure | pass           | yes         | unexpected success | OK    
                  | FAILED                     |
  | SkipTest        | AssertionError | no          | skipped            | OK    
                  | OK                         |
  | SkipTest        | any Exception  | no          | skipped            | OK    
                  | OK                         |
  | SkipTest        | pass           | no          | skipped            | OK    
                  | OK                         |
  | allowed_failure | AssertionError | yes         | skipped            | OK    
                  | OK                         |
  | allowed_failure | any Exception  | yes         | skipped            | OK    
                  | OK                         |
  | allowed_failure | pass           | yes         | pass               | OK    
                  | OK                         |
  |
  
  Whereas the current implementation is ignoring some conditions in PY >= 3.4 
but PY2 behaviour is the default one from above:
  
  | **Decorator**   | Exception      | **Testing** | **Behavior** | **Test 
result** |
  | expectedFailure | AssertionError | yes         | pass         | OK          
    |
  | expectedFailure | any Exception  | yes         | error        | FAILED      
    |
  | expectedFailure | pass           | yes         | pass         | OK          
    |
  | SkipTest        | AssertionError | no          | skipped      | OK          
    |
  | SkipTest        | any Exception  | no          | skipped      | OK          
    |
  | SkipTest        | pass           | no          | skipped      | OK          
    |
  | allowed_failure | AssertionError | yes         | skipped      | OK          
    |
  | allowed_failure | any Exception  | yes         | skipped      | OK          
    |
  | allowed_failure | pass           | yes         | pass         | OK          
    |
  |
  
  As described e.g. in T129368 <https://phabricator.wikimedia.org/T129368> 
expectedFailure indicates a test which currently fails because there is an 
unfixed bug. It counts as failure if the test fails but the test result is OK. 
If the test was sucessfull it is counted as unexpected success and the test 
fails in PY >= 3.4 (see good reasons above). Our implementation currently 
ignores counting failures and unexpected success in Python 3.4+ but just passes 
and the test result is OK even for the unexpected success condition.
  
  SkipTest or any other skip method is to be used if the test is not related to 
the current environment. It just skips the test.
  
  allowed_failure is a weird implementation. It always passes silently and 
there is no indication whether a test raises an error or fails and prohibits 
that bugs are solved.
  
  Regarding script_test: it must be rewritten due to T223032 
<https://phabricator.wikimedia.org/T223032> and I was not digging deeper here. 
Skipping is one alternative if I know that a test is not appropriate for a 
given script.

TASK DETAIL
  https://phabricator.wikimedia.org/T223030

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Xqt
Cc: Aklapper, pywikibot-bugs-list, Framawiki, Dvorapa, Dalba, Xqt, joker88john, 
Viztor, DannyS712, CucyNoiD, NebulousIris, Wenyi, Gaboe420, Versusxo, 
Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Baloch007, 
Darkminds3113, Bsandipan, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, 
Tbscho, MayS, WSH1906, Lewizho99, Mdupont, JJMC89, Maathavan, Avicennasis, 
mys_721tx, jayvdb, Masti, Alchimista, Rxy
_______________________________________________
pywikibot-bugs mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs

Reply via email to