Skip to content

John6797/sc 7795/add better handling for description futures#281

Merged
johnwalz97 merged 10 commits intomainfrom
john6797/sc-7795/add-better-handling-for-description-futures
Jan 3, 2025
Merged

John6797/sc 7795/add better handling for description futures#281
johnwalz97 merged 10 commits intomainfrom
john6797/sc-7795/add-better-handling-for-description-futures

Conversation

@johnwalz97
Copy link
Contributor

Internal Notes for Reviewers

Adding some changes that didn't get merged in on the first pass for this ticket before the holidays.

External Release Notes

@johnwalz97 johnwalz97 added enhancement New feature or request internal Not to be externalized in the release notes labels Jan 2, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2025

PR Summary

This pull request refactors the process of building test results and generating descriptions in the validmind/tests/run.py module. The key changes include:

  1. Refactoring of build_test_result Function: The build_test_result function has been modified to accept a test_doc parameter instead of separate doc and description parameters. The generate_description parameter has been removed, and the description generation logic has been moved to the run_test function.

  2. Simplification of _run_composite_test and _run_comparison_test Functions: These functions have been updated to pass a test_doc to build_test_result instead of handling description generation within themselves. The generate_description parameter has been removed from these functions.

  3. Introduction of _run_test Helper Function: A new helper function _run_test has been introduced to encapsulate the logic of running a test and building a test result. This function is now used within the run_test function to streamline the process.

  4. Modification of run_test Function: The run_test function now uses the _run_test helper function to execute tests and build results. The description generation logic has been moved to the end of the function, where it is conditionally executed based on the generate_description flag.

These changes aim to improve code readability and maintainability by reducing redundancy and centralizing the test result building logic.

Test Suggestions

  • Test the build_test_result function to ensure it correctly builds a test result with the new test_doc parameter.
  • Verify that the _run_composite_test function correctly constructs composite test results without the generate_description parameter.
  • Check that the _run_comparison_test function handles test result building and description generation as expected.
  • Ensure that the _run_test helper function correctly executes a test and builds a test result.
  • Test the run_test function to confirm that it correctly generates descriptions when generate_description is set to true.
  • Perform regression testing to ensure that existing functionality is not broken by the refactoring.

Copy link
Contributor

@AnilSorathiya AnilSorathiya left a comment

Choose a reason for hiding this comment

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

LGTM

@johnwalz97 johnwalz97 merged commit b93dd45 into main Jan 3, 2025
6 checks passed
@johnwalz97 johnwalz97 deleted the john6797/sc-7795/add-better-handling-for-description-futures branch January 3, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants