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.

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

test: fix py2 inconsistency with regex ID #1686

Merged
merged 1 commit into from
Sep 3, 2019

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Sep 2, 2019

For some reason assertions about compiled regex fail with Python 2.

We don't know why, but at least now we make assertions about the patterns, and not the compiled regex objects. This should prevent the false-negative on Travis CI (I hope).

@Exirel Exirel requested a review from dgw September 2, 2019 18:58
@Exirel Exirel added Bugfix Generally, PRs that reference (and fix) one or more issue(s) High Priority labels Sep 2, 2019
@Exirel Exirel added this to the 7.0.0 milestone Sep 2, 2019
@Exirel
Copy link
Contributor Author

Exirel commented Sep 2, 2019

Travis seems happy about these tests with Python 2.7.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Well, this will do to get us going again.

We have several PR builds that failed because of this problem, but not all. I'm at a complete loss as to why this is even a thing, to the point where I'm about ready to bring it up in the Travis-CI forums and/or ask for people to look at it in one of freenode's Python channels. Only the Travis environment seems able to trigger this particular problem.

Lest I forget to bring it up on IRC tomorrow (technically later today), I'll mention here: Why does @url add a list of compiled patterns to the callable, but @rule adds the patterns without compiling them? I think we should take a long look at why @url works the way it does, which is different from every other decorator, which attach only simple strings (or the occasional bool) and let the loader code turn those strings into usable regex objects. Probably only the loader should do any pattern compilation.

Separately, maybe we should consider using sets instead of lists?

>>> ['first', 'second'] == ['second', 'first']
False
>>> set(['first', 'second']) == set(['second', 'first'])
True

Wouldn't solve the weird issue that we had to deal with here, but maybe worth a long-term look anyway for other reasons.

That's enough issue-hijacking for now. 😂

@dgw dgw merged commit d0c0828 into sopel-irc:master Sep 3, 2019
@dgw
Copy link
Member

dgw commented Sep 3, 2019

@Exirel Thanks for putting this together so quickly! 👌

dgw referenced this pull request Sep 3, 2019
currency: switched crypto api and combined rates to allow cross-lookup
@Exirel Exirel deleted the fix-test-url-patterns-py2 branch September 5, 2019 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s) Build High Priority Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants