Skip to content

extract_descriptions.py should support test functions as well #124

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

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

cachafla
Copy link
Contributor

@cachafla cachafla commented Jun 25, 2024

Internal Notes for Reviewers

Ticket: Some test descriptions are empty

  • extract_descriptions.py should support test functions as well

External Release Notes

@cachafla cachafla added the internal Not to be externalized in the release notes label Jun 25, 2024
@cachafla cachafla requested a review from nrichers June 25, 2024 07:15
Copy link
Collaborator

@nrichers nrichers left a comment

Choose a reason for hiding this comment

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

I tested this PR locally with the documentation repo and it's a definite improvement but there are still some blank test descriptions, e.g. TrainingTestDegradation.

How I tested:

  1. Modify our documentation Makefile to clone this PR branch.
  2. Run make get-source which clones this branch and then generate the test descriptions.
  3. Run quarto preview to check the output.
  4. To verify there is an issue, also checked out this PR and ran poetry run python scripts/extract_descriptions.py validmind/tests locally — same issue.

You can see the results of my test here, including screenshots: validmind/documentation#229

@cachafla
Copy link
Contributor Author

@nrichers could you give it another try? I made additional fixes and it should now capture all edge cases.

Copy link
Collaborator

@nrichers nrichers left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀🚀🚀

Tested against documentation repo again — looks like all test descriptions are now there. Thank you for fixing!

@cachafla cachafla merged commit 6db68fc into main Jun 28, 2024
3 checks passed
@cachafla cachafla deleted the cachafla/sc-5078/some-test-descriptions-are-empty branch June 28, 2024 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Not to be externalized in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants