Skip to content

Conversation

@xross
Copy link
Contributor

@xross xross commented Aug 22, 2025

In order to attempt to make ComparisonTester a bit more friendly when debugging - and avoid copying the printing code everywhere (like this https://github.com/xmos/lib_gpio/blob/develop/tests/helpers.py) - I have added a verbosity param to the constructor.

This is an int (rather than bool) - so we can directly wire it up to verbosity of pytest (so we dont have to change test code to get more debug info out) e.g.

@pytest.fixture
def verbosity(pytestconfig):
    return (pytestconfig.getoption("verbose") or 0)

usage:

testPrayForPass(verbosity):

    ...

    tester = testers.ComparisonTester(open(expected_file), regexp=True, ordered=False, verbosity=verbosity)`
  
   pytest.run_on_simulator(.... tester=tester... )

then

pytest still gives stuff like

test_output.py:41: AssertionError
ERROR: Line 11 of output not found in expected
  Actual  : Checker complete
Fail

and pytest -v gives more verbose output like

GOLDEN: Checker complee
OUTPUT: Checker complete
-------------------------------------------- Captured stderr call -------------
ERROR: Line 11 of output not found in expected
  Actual  : Checker complete
Fail

Its only a relatively small change here: #52

I'm sure we can pretty the output a bit, but be nice to have it used from one place - if it meets peoples requirements?

If people could check it/review it that would be super - put "everyone" on review as its quite a common area of complaint and just to let people make people aware of it really

@xross xross requested a review from Copilot August 22, 2025 13:14

This comment was marked as outdated.

@xross xross force-pushed the feat/verbose_comparison_checker branch 2 times, most recently from 3eb1b4e to ee26e3c Compare August 22, 2025 13:27
@xross xross force-pushed the feat/verbose_comparison_checker branch from ee26e3c to 4514bf9 Compare August 22, 2025 13:29
@xross xross requested review from Copilot and shuchitak August 22, 2025 13:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds verbosity support to the ComparisonTester class to improve debugging capabilities when tests fail. The verbosity parameter allows users to control debug output level and can be wired directly to pytest's verbosity settings.

  • Adds a verbosity parameter to the ComparisonTester constructor
  • Implements verbose output showing both expected and actual lines during comparison
  • Modifies failure behavior to continue processing in verbose mode for better debugging

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@shuchitak shuchitak left a comment

Choose a reason for hiding this comment

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

this is a good change. I tested this with a sim test in lib_xua and it works as expected.
I'm unsure which is more useful - print the golden and actual outputs separately allowing the user to diff them in a separate editor if they wish - or interleaving them to show exactly what's missing .
However this is definitely a step in the right direction and way better than what we currently have!

@xross
Copy link
Contributor Author

xross commented Aug 26, 2025

this is a good change. I tested this with a sim test in lib_xua and it works as expected. I'm unsure which is more useful - print the golden and actual outputs separately allowing the user to diff them in a separate editor if they wish - or interleaving them to show exactly what's missing . However this is definitely a step in the right direction and way better than what we currently have!

Thank you for the feedback :)

@shuchitak
Copy link
Contributor

Tested this on a failing lib_xud test and the verbose output is quite helpful, as opposed to the 'ERROR: Length of expected output less than output' that I would otherwise get!

GOLDEN: Packet: HOST -> DEVICE
OUTPUT: Packet: HOST -> DEVICE
GOLDEN: PID: OUT (0xe1)
OUTPUT: PID: OUT (0xe1)
GOLDEN: Packet: HOST -> DEVICE
OUTPUT: Packet: HOST -> DEVICE
GOLDEN: PID: DATA1 (0x4b)
OUTPUT: PID: DATA1 (0x4b)
GOLDEN: Test done
OUTPUT: Test done
GOLDEN:
OUTPUT: ERROR: Test timed out
--------------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------------
ERROR: Length of expected output less than output
ERROR: Line 89 of output does not match expected
Expected:
Actual : ERROR: Test timed out
Fail

@humphrey-xmos
Copy link

humphrey-xmos commented Aug 28, 2025

Tested this on a failing lib_xud test and the verbose output is quite helpful, as opposed to the 'ERROR: Length of expected output less than output' that I would otherwise get!

GOLDEN: Packet: HOST -> DEVICE OUTPUT: Packet: HOST -> DEVICE GOLDEN: PID: OUT (0xe1) OUTPUT: PID: OUT (0xe1) GOLDEN: Packet: HOST -> DEVICE OUTPUT: Packet: HOST -> DEVICE GOLDEN: PID: DATA1 (0x4b) OUTPUT: PID: DATA1 (0x4b) GOLDEN: Test done OUTPUT: Test done GOLDEN: OUTPUT: ERROR: Test timed out --------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------- ERROR: Length of expected output less than output ERROR: Line 89 of output does not match expected Expected: Actual : ERROR: Test timed out Fail

That is a help, but wouldn't you get the same result if instead of
assert len(capture) >= len(expected)

You use?
assert capture == expected

Then it woudn'y matter about verbosity level.

@shuchitak
Copy link
Contributor

Tested this on a failing lib_xud test and the verbose output is quite helpful, as opposed to the 'ERROR: Length of expected output less than output' that I would otherwise get!
GOLDEN: Packet: HOST -> DEVICE OUTPUT: Packet: HOST -> DEVICE GOLDEN: PID: OUT (0xe1) OUTPUT: PID: OUT (0xe1) GOLDEN: Packet: HOST -> DEVICE OUTPUT: Packet: HOST -> DEVICE GOLDEN: PID: DATA1 (0x4b) OUTPUT: PID: DATA1 (0x4b) GOLDEN: Test done OUTPUT: Test done GOLDEN: OUTPUT: ERROR: Test timed out --------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------- ERROR: Length of expected output less than output ERROR: Line 89 of output does not match expected Expected: Actual : ERROR: Test timed out Fail

That is a help, but wouldn't you get the same result if instead of assert len(capture) >= len(expected)

You use? assert capture == expected

Then it woudn'y matter about verbosity level.

We allow regex based comparison and also unordered (ignore ordering) comparisons so assert capture == expected wouldn't work.

@xross xross merged commit bece044 into develop Sep 16, 2025
1 of 2 checks passed
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.

4 participants