Skip to content
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

chore: replace TestCase with pytest #89

Closed
wants to merge 1 commit into from

Conversation

gretelliz
Copy link

Closes #88

Summary

Replaced TestCase with pytest test suite or test case

Local Tests

Not needed

End-to-End Tests

Not needed

@gretelliz gretelliz requested a review from a team as a code owner January 17, 2024 19:24
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

@gretelliz, it's a good start. But notice, there was a regression of 30% in terms of unit test coverage. In the warning messages it's pointing out a conflict with __init__ being called on TestStruct.

tests/unit/v0x01/test_asynchronous/test_flow_removed.py:8
  /home/scrutinizer/build/tests/unit/v0x01/test_asynchronous/test_flow_removed.py:8: PytestCollectionWarning: cannot collect test class 'TestFlowRemoved' because it has a __init__ constructor (from: tests/unit/v0x01/test_asynchronous/test_flow_removed.py)
    class TestFlowRemoved(TestStruct):

A lot of Test Suites were inherinting from TestStruct, but since pytest won't allow __init__ to be overwritten this will need to be refactored to be compatible. If you take a look on TestStruct, essentially it's just providing convenience to set a particular struct object and then run a few common tests.

I believe the path with least resistance here would to refactor TestStruct as StructTest, and then on each test suite that used to be a sub class from TestStruct, you no longer subclass it, but then just define a test_base or whatever and then reuse all the current setters and then you call run_tests() - which would be a new method on TestStruct that would call every expected base test_* methods on StructTest, essentially we'd be losing part of the automatic test collection that we used to have with the inheritance but I believe it's a decent compromise considering this refactor, here's an example:

class TestHello:
    """Hello message tests (also those in :class:`.TestDump`)."""

    def test_base(self):
        """Configure raw file and its object in parent class (TestDump)."""
        struct_test = StructTest()
        struct_test.set_raw_dump_file('v0x04', 'ofpt_hello')
        struct_test.set_raw_dump_object(Hello, xid=62,
                                        elements=_new_list_of_elements())
        struct_test.set_minimum_size(8)
        struct_test.run_tests()

The pytest way is typically with fixtures to do this sort of thing, but from what I'm initially seeing here it would be much harder to do. Let me know if you're seeing something simpler.

@viniarck
Copy link
Member

viniarck commented Feb 8, 2024

I'll convert this to draft since there has been no updates on this PR.

@viniarck viniarck marked this pull request as draft February 8, 2024 17:09
@viniarck
Copy link
Member

viniarck commented Feb 9, 2024

Closing this PR in favor of #90

@viniarck viniarck closed this Feb 9, 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.

chore: Replace TestCase with pytest test suite or test case
2 participants