Skip to content

Conversation

@thaJeztah
Copy link
Member

Some changes I had locally (got some more, but in a follow-up) 😄

move digest tests to root

These tests have to be in their own package to prevent a circular import
between testdigest and the main module, but Go allows for blackbox-testing
by using a "_test" package.

This patch:

  • moves the test file to be next to the implementation.
  • removes the redundant imports of "crypto/sha256" and "crypto/shasha512",
    as they are imported by default since 084376b

TestFroms: fix some linting issues

Keep the linters happy:

  • fix an unhandled error (but unlikely to happen)
  • fix some variables being shadowed

TestFroms: use sub-tests

Use sub-tests to not fail early if an algorithm fails.

With this change:

=== RUN   TestFroms
=== RUN   TestFroms/sha256
=== RUN   TestFroms/sha384
=== RUN   TestFroms/sha512
--- PASS: TestFroms (0.02s)
    --- PASS: TestFroms/sha256 (0.01s)
    --- PASS: TestFroms/sha384 (0.00s)
    --- PASS: TestFroms/sha512 (0.00s)
PASS

gofmt, and touch-up some godoc

These tests have to be in their own package to prevent a circular import
between testdigest and the main module, but Go allows for blackbox-testing
by using a "_test" package.

This patch:

- moves the test file to be next to the implementation.
- removes the redundant imports of "crypto/sha256" and "crypto/shasha512",
  as they are imported by default since 084376b

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Keep the linters happy:

- fix an unhandled error (but unlikely to happen)
- fix some variables being shadowed

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use sub-tests to not fail early if an algorithm fails.

With this change:

    === RUN   TestFroms
    === RUN   TestFroms/sha256
    === RUN   TestFroms/sha384
    === RUN   TestFroms/sha512
    --- PASS: TestFroms (0.02s)
        --- PASS: TestFroms/sha256 (0.01s)
        --- PASS: TestFroms/sha384 (0.00s)
        --- PASS: TestFroms/sha512 (0.00s)
    PASS

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah marked this pull request as ready for review August 1, 2023 13:44
@dmcgowan dmcgowan merged commit 122dc63 into opencontainers:master Aug 1, 2023
@thaJeztah thaJeztah deleted the move_tests branch August 1, 2023 15:03
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.

3 participants