| Dalba added a comment. |
In T185317#3913727, @divadsn wrote:@Dalba ok, that's a point, but now why the revert?
That patch could be fixed with a follow-up, which would be simply adding a |Invalid ISBN found to the regex, why not instead?
IMO, https://gerrit.wikimedia.org/r/#/c/399348/ was more deserved to be merged. It was submitted before your patch and it did not have this issue.
Another issue is the usage of | between the regular _expression_, which seems too lenient to me. We could try using a regular _expression_ according to available imports. It could simplify the job of finding the changed _expression_ in case upstream changes. I have not thought this through, though.
Just so that you know, I am also doubtful about the whole idea of always using assertRaisesRegex instead of assertRaises. I think that at least sometimes using assertRaisesRegex instead of assertRaises makes tests unnecessarily fragile against changes upstream. If assertRaises was really that bad, python core developers would have deprecated it... I know, this is not the place to discuss this, and I'm fine if others that find it useful continue with that task.
Cc: Albert221, divadsn, gerritbot, Aklapper, pywikibot-bugs-list, Dalba, Adrian1985, Cpaulf30, Baloch007, Darkminds3113, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, Magul, Tbscho, rafidaslam, MayS, Lewizho99, Mdupont, JJMC89, Maathavan, Avicennasis, jayvdb, Masti, Alchimista, Rxy
_______________________________________________ pywikibot-bugs mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/pywikibot-bugs
