Skip to content

Conversation

@wkukka1
Copy link
Contributor

@wkukka1 wkukka1 commented May 28, 2025

changed python test names to appear as [filename] testname.

image
results from running the autotest_py tests^

@wkukka1 wkukka1 requested a review from david-yz-liu May 28, 2025 14:38
@wkukka1 wkukka1 marked this pull request as draft May 28, 2025 14:39
@wkukka1 wkukka1 closed this May 28, 2025
@wkukka1 wkukka1 reopened this May 28, 2025
@wkukka1 wkukka1 marked this pull request as ready for review May 28, 2025 15:41
Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@wkukka1 good work! However, it's probably better to parse the test's node id, for a few reasons:

  1. Prefer keeping the test relative path (rather than just filename)
  2. When the unit test is defined in a class rather than at the top level (in this case, I'd still like the class name to be displayed)
  3. Avoid accessing protected attributes

@wkukka1
Copy link
Contributor Author

wkukka1 commented May 28, 2025

Thanks for the feedback!

For the unit tests should the format be [filename] classname.funcname

@david-yz-liu
Copy link
Contributor

For the unit tests should the format be [filename] classname.funcname

Yes that's good 👍

@wkukka1 wkukka1 requested a review from david-yz-liu May 28, 2025 19:13
@wkukka1
Copy link
Contributor Author

wkukka1 commented May 28, 2025

Updates:

  • Refactored the logic for generating test names in both unittest and pytest

  • For unittest, test names are now derived from test.id() in [relative/path/to/file.py] classname.funcname format, removing the use of protected attributes

  • For pytest, improved nodeid parsing to support both function-based and class-based test definitions

  • The name formatting logic is now more resilient to different directory structures and environments

  • Updated the tests to use regex when asserting formatted test names to support variations in relative paths (e.g., in cloud or CI environments)

image
^updated unit test names to show classname

Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@wkukka1 Okay this is great. Two additional comments:

  1. Please now make the same changes for the Jupyter tester.
  2. Please update the Changelog.

@donny-wong donny-wong added this to the v2.7.1 milestone May 29, 2025
@wkukka1
Copy link
Contributor Author

wkukka1 commented May 29, 2025

Updates:

  • updated jupyter notebook name to follow [filename] funcname
  • created unit test for jupyter naming conventions
  • updated the ci workflow to install required packages for jupyter tester to be used

image
^ updated names

@wkukka1 wkukka1 requested a review from david-yz-liu May 29, 2025 15:59
@david-yz-liu david-yz-liu merged commit 50f825e into MarkUsProject:master May 29, 2025
7 checks passed
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.

3 participants