Skip to content

feat(op-acceptor): handle package tests better.#225

Merged
teddyknox merged 1 commit intomainfrom
scharissis/op-acceptor/package-tests-fix
Mar 13, 2025
Merged

feat(op-acceptor): handle package tests better.#225
teddyknox merged 1 commit intomainfrom
scharissis/op-acceptor/package-tests-fix

Conversation

@scharissis
Copy link
Copy Markdown
Contributor

@scharissis scharissis commented Mar 12, 2025

Description

Show the result of each sub-test within a package in the output. Also show the package name.

Test
Added some unit tests. Also made a custom validators.yaml with just a single package which has 4 subtests. The output was as expected (pictured).

Screenshot 2025-03-12 at 13 57 56

Metadata

@scharissis scharissis requested a review from teddyknox March 12, 2025 02:58
@scharissis scharissis self-assigned this Mar 12, 2025
@scharissis scharissis requested a review from a team as a code owner March 12, 2025 02:58
@scharissis scharissis requested a review from edobry March 12, 2025 02:58
Comment on lines +309 to +300
if displayName == "" && test.Metadata.Package != "" {
// For package names, use a shortened version to avoid wrapping
pkgParts := strings.Split(test.Metadata.Package, "/")
if len(pkgParts) > 0 {
displayName = pkgParts[len(pkgParts)-1] + " (package)"
} else {
displayName = test.Metadata.Package
}
}

// Display the test result
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small nit: these two blocks are identical to the ones above, I'd maybe just DRY into some private function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call; I've now fixed all these up into one function. Cheers!

Comment on lines +574 to +576
// Get a display name for the test
displayName := testName
if displayName == "" && test.Metadata.Package != "" {
// For package names, use a shortened version to avoid wrapping
pkgParts := strings.Split(test.Metadata.Package, "/")
if len(pkgParts) > 0 {
displayName = pkgParts[len(pkgParts)-1] + " (package)"
} else {
displayName = test.Metadata.Package
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

Comment on lines +619 to +612
// Get a display name for the test
displayName := testName
if displayName == "" && test.Metadata.Package != "" {
// For package names, use a shortened version to avoid wrapping
pkgParts := strings.Split(test.Metadata.Package, "/")
if len(pkgParts) > 0 {
displayName = pkgParts[len(pkgParts)-1] + " (package)"
} else {
displayName = test.Metadata.Package
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

Show the result of each sub-test within a package in the output. Also show the package name.
@scharissis scharissis force-pushed the scharissis/op-acceptor/package-tests-fix branch from 3845c75 to 4d256fb Compare March 12, 2025 19:58
@teddyknox teddyknox merged commit e102113 into main Mar 13, 2025
6 checks passed
@teddyknox teddyknox deleted the scharissis/op-acceptor/package-tests-fix branch March 13, 2025 20:22
raffaele-oplabs pushed a commit that referenced this pull request Aug 3, 2025
Show the result of each sub-test within a package in the output. Also show the package name.
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.

op-acceptance-tests output does not handle package-level test declarations

4 participants