Skip to content

test(report): switch to testing report Merge instead of FromString #5216

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 1 commit into from
May 28, 2025

Conversation

mcwarman
Copy link
Member

@mcwarman mcwarman commented May 28, 2025

When working on #5189 I found it difficult to update the tests as it relied on the correct string indentation.

Using https://github.com/sanity-io/litter to get the objects out of the existing tests, I've converted all the FromString tests to .Merge tests. This made it easier to update tests.

Also checked coverage and added a new Changelog scenarios where the new and old report have more items respectively.

Also added a test for ToActionsString as a part of existing test ensuring the object round trips.

Test

To test this pull request, you can run the following commands:

cd pkg/core/reports
go test

Additional Information

Checklist

  • I have updated the documentation via pull request in website repository.

Tradeoff

Potential improvement

@mcwarman mcwarman requested a review from olblak May 28, 2025 12:21
@mcwarman mcwarman changed the title test(report): switch to testing report merge test(report): switch to testing report Merge instead of FromString May 28, 2025
@mcwarman mcwarman force-pushed the test/object-tests branch from 4e0bc1d to 101ce06 Compare May 28, 2025 12:21
@mcwarman mcwarman marked this pull request as ready for review May 28, 2025 12:23
@olblak olblak merged commit a09c19d into updatecli:main May 28, 2025
6 checks passed
@mcwarman mcwarman deleted the test/object-tests branch May 28, 2025 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants