Skip to content

[Tigron]: mocks + debugging output enhancements #4076

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

Merged
merged 3 commits into from
Apr 12, 2025

Conversation

apostasie
Copy link
Contributor

@apostasie apostasie commented Apr 3, 2025

<!> Blocked by <!>:

Should preferably be merged after:


This is a new series of Tigron PRs (FIXME: currently on top of #4056).

Now that the big change with the new command implementation is in, upcoming work on Tigron will focus on:

  • P0: internal testing, getting to the point we can absolutely trust Tigron to never faceplant
  • P0: debugging output readability and other developers quality of life improvements
  • P1: code cleanup and simplification

Ideally, none of these PRs will require ANY change in nerdctl, and are either changes to the way we test Tigron itself, or refactoring that does not change signatures and API.

Efforts are being made to break these PRs down into small, digestible changes to ease and fasten review, which explains the chain of PRs.

LMK at any point if a given PR needs to be further broken down.


This first PR is focused on making Tigron more testable, starting with assertive.

Commit 1 is mundane and just disables golangci linters that we do not want.

Second commit does introduce internal/mimicry, a mock mechanism that will allow mocking *testing.T and other conendrums. See commit message for details.

Third commit is work on internal/assertive (code cleanup, methods generalization), and a raft of new tests (leveraging mimicry). See commit message for details

See following comments for preview of how failures will now be presented.

Note that more work on debugging output quality will very soon come in an upcoming PR (this time focused on general output, which is currently crowded, noisy and really hard to read) - I would prefer to keep that separate to keep PRs small, though meanwhile the display will be a bit loosy goosy because of that.

@apostasie apostasie force-pushed the tigron-1-mock branch 3 times, most recently from 88b9c53 to 7008b01 Compare April 3, 2025 19:16
@apostasie
Copy link
Contributor Author

apostasie commented Apr 3, 2025

Here is how the new assertive debug will be presented.

The first line is enriched with the exact content from the source test file that triggered the failure (to facilitate copy paste search), as a clickable OCS8 link so that the (local) developer can directly click and open the offending file.

The following lines present clearly what was expected, what was the actual value, with some high-contrast emojis to facilitate reading (for old people like me).

Screenshot 2025-04-03 at 12 18 27 PM

This is to be compared with the normal "assert.Assert" information:

Screenshot 2025-04-03 at 12 23 08 PM

@apostasie
Copy link
Contributor Author

apostasie commented Apr 3, 2025

Here is how the captured call traces will show when using mocks:

In that specific case, the test wants to make sure that the mocked method FailNow was NOT called.
Since the method was called, the test fails, and shows the call stack from where the offending call was made, along with the arguments that were passed to the function (well, [] in that case...) and the call time.

Source links are clickable as well.

Screenshot 2025-04-03 at 12 17 30 PM

This of course is not replay-debugging - just a frame capture done with runtime.

@apostasie apostasie changed the title [DNR] [WORK IN PROGRESS] [BLOCKED] [Tigron]: mocks and debugging output enhancements Apr 3, 2025
@apostasie apostasie marked this pull request as ready for review April 3, 2025 21:18
@apostasie apostasie changed the title [BLOCKED] [Tigron]: mocks and debugging output enhancements [BLOCKED] [Tigron]: mocks + debugging output enhancements Apr 3, 2025
@apostasie apostasie changed the title [BLOCKED] [Tigron]: mocks + debugging output enhancements [STACKED] [Tigron]: mocks + debugging output enhancements Apr 3, 2025
@apostasie apostasie changed the title [STACKED] [Tigron]: mocks + debugging output enhancements [Tigron]: mocks + debugging output enhancements Apr 8, 2025
@apostasie
Copy link
Contributor Author

Rebased.
This is ready for review.

@AkihiroSuda AkihiroSuda added this to the v2.0.5 milestone Apr 8, 2025
@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Apr 8, 2025
This commit relaxes line length from 100 to 120, and disables a bunch of linters we do not want.

Signed-off-by: apostasie <[email protected]>
Mimicry is a lightweight, zero dependency mock mechanism created to ease testing of Tigron.
Since Tigron heavily relies on *testing.T, it is currently hard to test.
Moving away to a tig.T interface instead of *testing.T will unlock the ability to mock.
Mimicry does provide:
- recording of all function calls, with arguments and complete stack trace (see Report())
- optional custom handling of function calls (see Register())
- QOL: fancyfied OCS8 links allow opening files from traces in terminal

UX is largely in flux right now and experimental, but the objective is to:
- do not require code generation
- do not abuse reflection
- keep the amount of boilerplate to the absolute minimum for the mock consumer
- ... and as small as possible for the mock creator
- keep zero dependencies

This commit also introduce the tig.T interface to be used everywhere inside Tigron in the future,
along with a complete mock for it.

Mimicry is not meant to be used directly for now, though, if there is interest, a future
version might graduate out of `internal`.

Signed-off-by: apostasie <[email protected]>
This brings a set of enhancements to assertive:
- function name simplifications
- generics-ifying of some functions that can be used on comparables
- additional methods (Match, DoesNotMatch)
- debugging output is made clear, along with OCS8 link and excerpt of the test file
- all methods can now elect to Fail later instead of FailNow
- Check method is removed (^ because of above)
- all methods can now elect to silence output on success

Other Tigron files are updated to adopt these changes:
- method names update
- comparators move away from Check and now fully leverages assertive

This also adds a large number of tests for assertive, leveraging mimicry.

Signed-off-by: apostasie <[email protected]>
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 625b674 into containerd:main Apr 12, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci e.g., CI failure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants