Skip to content

Fixed Auto Discovery of .test files Fixes #8650 #10108

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

Closed
wants to merge 2 commits into from

Conversation

MiHarsh
Copy link

@MiHarsh MiHarsh commented Feb 18, 2021

Fixes #8650

After this PR is merged, this while running testcheck.py , it would automatically create the list of .test files. The main benefit is that if in future there is addition of test files, we would not have to add the new file into testcheck.py manually, it would automatically grab them.

Further I have verified the changes on my local.

Please merge this PR. I am new to open source and this is my first PR.

Please review @JukkaL , @ethanhs and @davidzwa Sir


for check_files in os.listdir(path2loc):
# Append only if it has the pattern ( check-*-*.test)
if re.match(r'check-\w+-?\w+?.test', check_files):
Copy link
Contributor

@kamilturek kamilturek Mar 9, 2021

Choose a reason for hiding this comment

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

Thanks for your contribution.

FIlename check-union-or-syntax.test is not matched by this regex.

Copy link
Author

Choose a reason for hiding this comment

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

@kamilturek thanks for pointing this. I'll update the regex.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Sorry for the slow response! Left some thoughts about how to make this cleaner and more reusable.

if sys.platform == 'win32':
path2loc = os.path.abspath(__file__).split('mypy\\test')[0] + 'test-data\\unit'
else:
path2loc = os.path.join(os.path.abspath(__file__).split('mypy/test')[0], 'test-data/unit')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is hard to reason about. Can you unify the cases so that the same implementation works on both Windows and other platforms? You can probably use os.path.join and/or os.sep.

for check_files in os.listdir(path2loc):
# Append only if it has the pattern ( check-*-*.test)
if re.match(r'check-\w+-?\w+?-?\w+?.test', check_files):
typecheck_files.append(check_files)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you move this and the above logic to a helper module, so that this can be reused in other places easily?

For example, we could have something like this in this file:

typecheck_files = expand_test_files('test-data/unit/check-*.test')

You can use glob.glob to expand patterns instead of re, since regular expressions are more error-prone as they require escaping, etc.

@97littleleaf11
Copy link
Collaborator

Any progress on this PR? @MiHarsh

@hauntsaninja
Copy link
Collaborator

Superseded by #13143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-discovery of .test files
5 participants