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(testing): Add basic unit test for Services Scanner #496

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peckto
Copy link
Contributor

@peckto peckto commented Jan 11, 2024

This is a first draft of a unit test on scanner level.
The code uses pytest and the python inbuilt unittest.mock library.
Because of missing modularization, mocking the argparse library is necessary,
which will be fixed in the future via a config container.
Currently, the transport module is mocked, but we could also create an own transport module for testing purpose.
As we currently have no defined scanner result, changes in the scan-services scanner are included for demonstration purposes.

To handle most of the default requests, the vECU is used.
To define test case specific ECU behavior, a callback hook_request is provided, which can overwrite the response from the vECU.

Three different ECU behaviors are implemented:

  • PASS: forward response to the scanner (from the vECU or user defined)
  • RAISE: raise defined exception instead of forwarding response
  • TIMEOUT: simulate timeout during receive (realized as TimeoutError exception)

In the draft, three basic test cases for the scan-services scanner are included:

  1. Basic test of Service Scan against vECU
  2. Test TimeoutError on Service Scan
  3. Test NegativeResponse on Service Scan

You can run the test cases via pytest:

pytest -k TestScanServices -s

@@ -80,10 +80,10 @@ def configure_parser(self) -> None:
async def main(self, args: Namespace) -> None:
self.result: list[tuple[int, int]] = []
self.ecu.max_retry = 1
found: dict[int, dict[int, Any]] = {}
self.found: dict[int, dict[int, Any]] = {}
Copy link
Member

Choose a reason for hiding this comment

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

A unified interface for scanner results needs to be defined; this is not decided yet. So for these first tests, let's use your solution. We could adjust this later.



@dataclass
class Args:
Copy link
Member

Choose a reason for hiding this comment

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

For now ok; in gallia 2.0 this will be replaced by config containers.

"""
assert self.scanner is not None

async def handle(
Copy link
Member

Choose a reason for hiding this comment

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

As discussed via phone: This will be replaced via a table with TypedDicts, as suggested by russ cox: https://research.swtch.com/testing. Especially his tips 1, 5, 8, and 11 are relevant here.

Your draft from #495 could be used for a starting point.

assert self.scanner.found[1][0x27] == TimeoutError.__name__

@pytest.mark.asyncio
async def test_3(self, setup: None, teardown: None) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Find descriptive test case names


@pytest.mark.asyncio
@pytest.fixture
@patch("gallia.transports.unix.UnixTransport.connect")
Copy link
Member

Choose a reason for hiding this comment

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

Let's create a separate vecu transport for testing.

Copy link

stale bot commented Jul 14, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 14, 2024
@rumpelsepp rumpelsepp added this to the gallia 2.0 milestone Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants