Skip to content

adding 'only' option to the tests.py#248

Merged
ipdemes merged 2 commits intonv-legate:branch-22.05from
ipdemes:tests_only
Apr 6, 2022
Merged

adding 'only' option to the tests.py#248
ipdemes merged 2 commits intonv-legate:branch-22.05from
ipdemes:tests_only

Conversation

@ipdemes
Copy link
Contributor

@ipdemes ipdemes commented Mar 31, 2022

No description provided.

@ipdemes ipdemes requested a review from manopapad March 31, 2022 04:06
@ipdemes ipdemes mentioned this pull request Mar 31, 2022
legate_tests.extend(glob.glob("tests/interop/*.py"))
legate_tests.extend(glob.glob("tests/interop/*.py")) # noqa: F823

if only_pattern is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not repeat this, unless there's a reason good reason to do so. Can't you factor this and the same code above out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done

test.py Outdated
):
if interop_tests:
legate_tests.extend(glob.glob("tests/interop/*.py"))
legate_tests.extend(glob.glob("tests/interop/*.py")) # noqa: F823
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to override F823? We generally don't want overrides like this, unless there's no way around it. (as far as I know we use this override only for very long function names.)

test.py Outdated
):
if interop_tests:
legate_tests.extend(glob.glob("tests/interop/*.py"))
legate_tests.extend(glob.glob("tests/interop/*.py")) # noqa: F823
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to override F823? We generally don't want overrides like this, unless there's no way around it. (as far as I know we use this override only for very long function names.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was a leftover, removed

@ipdemes
Copy link
Contributor Author

ipdemes commented Apr 6, 2022

@magnatelee : can I merge this?

@magnatelee
Copy link
Contributor

Yes. Please merge it when you can.

@ipdemes ipdemes merged commit dc6f929 into nv-legate:branch-22.05 Apr 6, 2022
@ipdemes ipdemes deleted the tests_only branch May 26, 2022 20:20
manopapad pushed a commit that referenced this pull request Nov 17, 2024
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.

2 participants