[Tigron]: expect.JSON comparator and updated volume test #4079
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apostasie
force-pushed
the
tigron-3-json-comparator
branch
7 times, most recently
from
9f2f884 to
b9eecf1
Compare
apostasie
changed the title
32 tasks
apostasie
force-pushed
the
tigron-3-json-comparator
branch
from
b9eecf1 to
8afa7ab
Compare
|
Rebased. |
apostasie
force-pushed
the
tigron-3-json-comparator
branch
4 times, most recently
from
e924b07 to
d84e7d1
Compare
apostasie
changed the title
|
Rebased & ready for review. |
|
#4046 is getting annoying.
So, either the timeout is just too short (3 seconds) for the size of our GHA instances: https://github.com/ktock/buildg/blob/main/pkg/buildkit/client.go#L83 ... or something else gets stuck in some cases. Exploring a hunch and a workaround in #4103. |
1 task
AkihiroSuda
reviewed
Apr 14, 2025
AkihiroSuda
reviewed
Apr 14, 2025
AkihiroSuda
requested changes
Apr 14, 2025
apostasie
force-pushed
the
tigron-3-json-comparator
branch
from
d84e7d1 to
d039311
Compare
This changeset adds expect.JSON[T any], which will allow easier testing of json output and hopefully remove a lot of boilerplate unmarshalling / assert in tests. It also adds an extensive doc.md document about comparators (plan is trim down the main documentation to a small set of simple examples, then link to these "advanced" docs for further reading), which will allow for easier documentation maintenance and more approachable reading. Signed-off-by: apostasie <[email protected]>
Signed-off-by: apostasie <[email protected]>
In some cases, exec is just really slow. Adjusting tests so that we start counting after the command actually started. Signed-off-by: apostasie <[email protected]>
apostasie
force-pushed
the
tigron-3-json-comparator
branch
from
d039311 to
9255358
Compare
|
Thanks @AkihiroSuda |
|
CI not running? |
Yeah I saw that the other day on another PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
<!> Blocked by <!>:
This is a new series of Tigron PRs.
Now that the big change with the new command implementation is in, upcoming work on Tigron will focus on:
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 third PR is small as well (most of it is documentation)
Commit 1 adds support for
expect.JSON[T].The point is to remove repeated json unmarshalling boilerplate from tests.
Parsing JSON of course is not complex, but it adds unnecessary noise to tests.
Also added a detailed doc.
Second commit does change a couple of tests in nerdctl to demonstrate usage on volume tests.
If we are happy with the direction this is going to, I will cleanup more helpers and tests in a follow-up PR.
Third commit is unrelated to
expect.JSON, but does fix a condition that triggered a failure on this specific PR, and is a minor test adjustment.