Enhancement: Refactor Test Code for Efficiency and Quality

[freddiewanah]

Description

As a first-time contributor with a background in researching Python unit tests, I conducted a thorough static analysis of the project's test code. This examination revealed various "test smells" that could potentially degrade both the efficiency and quality of our tests. To address these issues, I've refactored the relevant code segments. The modifications I propose not only aim to eliminate these inefficiencies but are also expected to reduce execution time within GitHub Actions. This reduction could contribute to lower operational costs for the project. Additionally, I've identified three issues within the test code that could cause problems during CICD.

Three issues:

• tests/test_pad_collation.py line 120: supposed to be assertEqual, but not assertTure? or it will always be true.
• tests/test_to_numpy.py line 74: Same as above.
• tests/test_auto3dseg.py line 370: typo? two same asserts next to each other.

Test smells refactored:

• Suboptimal Assert: Instead of using statements such as assertIsNone, assertIsInstance, always simply use assertTrue or assertFalse. This will decrease the code overall readability and increase the execution time as extra logic needed.
• Exception Handling: Instead of catch exceptions with assertRaise, use try/except commands. This will cause extra execution time according to the experimental tests.
• Duplicate Assert: Having same asserts in one test case. Having multiple asserts in one test cases can cause potential issues like when the first assert fails, the test case stops and won't check the second assert. By using @parameterized.expand, this issue can be resolved and the caching also saves execution time.
• Similar Test Methods: Test methods within the same test class that have similar code. Refactor such cases with @parameerized.expand can not only reduce code redundancy, but also saves execution time by caching similar code base.

I didn't change any of the testing logic or testing values. The refactoring only focuses on reducing the test smells.

Please feel free to let me know if you are interested in more refactoring like this or if there're any changes that you don't wish.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[atbenmurray]

Hi @KumoLiu, I'll take a look through the changes.
Hi @freddiewanah, thanks for the PR; I'll have a look over everything.

[atbenmurray]

Hi @freddiewanah. We discussed this PR in our regular meeting and we'd like to request that it gets broken up into smaller, more manageable pieces rather than a monolithic PR that touches 65 different files. Ideally, it should be as follows:

1. test_pad_collation issue, test_to_numpy issue, test_auto3dseg.py issue
2. Adapting tests to @parameterized
3. Suboptimal asserts (potentially split up into smaller related PRs
4. assertRaise changes (although we may not go with these in the end)

Would this be ok?
Thanks,
Ben

[freddiewanah]

Hi @atbenmurray thanks for the review. If I understand correctly, do you want me to break this PR into 4 PRs, where each of them addresses one smell/issue you listed in the comment?

[atbenmurray]

Hi @atbenmurray thanks for the review. If I understand correctly, do you want me to break this PR into 4 PRs, where each of them addresses one smell/issue you listed in the comment?

Yes, that's correct. It might also be worth splitting category 3 into a few smaller PRs. We prefer to avoid PRs that many files if they can be subdivided into smaller PRs.

[freddiewanah]

Closing this PR as the requested new PRs (#7672, #7672, #7670, #7663, #7662 ) were created. Thanks for the review.