Skip to content

Python: Fix (some) shortcomings of tests filter #3212

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 27, 2020
Merged

Python: Fix (some) shortcomings of tests filter #3212

merged 3 commits into from
Apr 27, 2020

Conversation

RasmusWL
Copy link
Member

@RasmusWL RasmusWL commented Apr 7, 2020

I added (some) since I'm not convinced it's 100% perfect after this change 😉

@RasmusWL RasmusWL added the Python label Apr 7, 2020
@RasmusWL RasmusWL requested a review from a team as a code owner April 7, 2020 09:32
@RasmusWL RasmusWL requested review from BekaValentine and removed request for a team April 7, 2020 09:32
@RasmusWL
Python: Consistenly use TestScope instead of Test
46ecbef 
It's not clear which one is the correct to use, but there were more uses of
TestScope than Test, so I'm assuming that is the right one ¯\_(ツ)_/¯
@RasmusWL
Copy link
Member Author

RasmusWL commented Apr 7, 2020

It's still unclear to me what the meaning behind having both TestScope and Test is supposed to be. I really wanted to add a comment explaining what they both are, but simply could figure it out. I did not to a deep dive into our archives to see when they where added/modified.

From counting number of uses it looks like you're supposed to use TestScope, but that might just be accidental.

@RasmusWL RasmusWL mentioned this pull request Apr 7, 2020
Merged
@RasmusWL RasmusWL requested review from tausbn and removed request for BekaValentine April 27, 2020 08:39
@tausbn
Copy link
Contributor

tausbn commented Apr 27, 2020

I'm thinking maybe the intent was to not count UnitTestClasses as tests, in order to get a better count of the number of actual tests (in Metrics/FNumberOfTests.ql). This appears to be the only place (apart from the assert query you modernised) where Test is specifically mentioned.

tausbn
tausbn approved these changes Apr 27, 2020
View reviewed changes
Copy link
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.

This looks good to me. 👍

@tausbn tausbn merged commit de08433 into github:master Apr 27, 2020
@RasmusWL RasmusWL deleted the python-fix-tests-filter branch April 27, 2020 12:59
RasmusWL added a commit to RasmusWL/codeql that referenced this pull request Sep 8, 2020
@RasmusWL
Python: Add missing change notes
2979f98 
I looked through PRs between rc/1.24 and rc/1.25 and added missing change notes for:

- github#3314
- github#3302
- github#3212
- github#3453
- github#3407
- github#3563

```
git log --grep="Merge pull request" --format=oneline rc/1.24..rc/1.25 -- python/
```
@RasmusWL RasmusWL mentioned this pull request Sep 8, 2020
Merged
luchua-bc pushed a commit to luchua-bc/ql that referenced this pull request Oct 16, 2020
@RasmusWL @luchua-bc
Python: Add missing change notes
77ab5d5 
I looked through PRs between rc/1.24 and rc/1.25 and added missing change notes for:

- github#3314
- github#3302
- github#3212
- github#3453
- github#3407
- github#3563

```
git log --grep="Merge pull request" --format=oneline rc/1.24..rc/1.25 -- python/
```
@snyk-bot snyk-bot mentioned this pull request Jan 13, 2022
Open
@aliscco aliscco mentioned this pull request Sep 23, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tausbn tausbn tausbn approved these changes

Assignees
No one assigned
Labels
Python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RasmusWL @tausbn