Skip to content

Python: include zope web tests from internal repo#2482

Merged
tausbn merged 3 commits intogithub:masterfrom
RasmusWL:python-include-zope-web-tests
Dec 18, 2019
Merged

Python: include zope web tests from internal repo#2482
tausbn merged 3 commits intogithub:masterfrom
RasmusWL:python-include-zope-web-tests

Conversation

@RasmusWL
Copy link
Copy Markdown
Member

@RasmusWL RasmusWL commented Dec 2, 2019

As the only part of the web modernisation, there was not any dedicated web library support for Zope, only these tests.

We do have the python/ql/src/semmle/python/libraries/Zope.qll file, but that isn't even used in this test ¯\_(ツ)_/¯

@RasmusWL RasmusWL added depends on internal PR This PR should only be merged in sync with an internal Semmle PR Python labels Dec 2, 2019
@RasmusWL RasmusWL requested a review from tausbn December 2, 2019 13:55
@RasmusWL RasmusWL requested a review from a team as a code owner December 2, 2019 13:55
@RasmusWL RasmusWL changed the title Python: include zope web tests Python: include zope web tests from internal repo Dec 2, 2019
Copy link
Copy Markdown
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

One minor question, otherwise LGTM.

@@ -0,0 +1 @@
semmle-extractor-options: --max-import-depth=3 -p ../../../query-tests/Security/lib/ --respect-init=False
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We removed --respect-init=False when moving the twisted tests. I'm wondering if it could be removed here as well, without affecting the tests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. I'm quite sure I was looking at some zope source code and noticed that they didn't have an zope/__init__.py file, which I why I did it like this. However, checking their codebase now I can see that they do in fact have a zope/__init__.py file. Fixed it up, and tests passed locally 👍

@RasmusWL RasmusWL force-pushed the python-include-zope-web-tests branch from f4c857c to 8b5d6ae Compare December 17, 2019 16:42
@tausbn tausbn merged commit eb6feee into github:master Dec 18, 2019
@RasmusWL RasmusWL deleted the python-include-zope-web-tests branch December 18, 2019 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

depends on internal PR This PR should only be merged in sync with an internal Semmle PR Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants