Skip to content
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

Snapshots combined with using -count #87

Closed
G-Rath opened this issue Jan 9, 2024 · 5 comments · Fixed by #90
Closed

Snapshots combined with using -count #87

G-Rath opened this issue Jan 9, 2024 · 5 comments · Fixed by #90
Labels
question Further information is requested

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Jan 9, 2024

Issue

This could be a bug or a feature - I'm still actually digging into this but wanted to open an issue because you might be able to dig into it faster (if you've not already come across this!)

It looks like when running with -count (i.e. go test -count 5 ./...) in some (if not all?) cases snapshots get counted multiple times for the same test rather than recompared - i.e. the snapshot gets updated with [TestRun/#00 - n] where n is the number of snapshots called so far across all runs.

I think this is because the test ids are using a global registry + mutex and that -count is working within the same process, it just calls the test functions multiple times.

I think this could be worked around by exposing a way of manually clearing/resetting the registry, which could be called before every test in TestMain, but I've not yet tested that to confirm.

I'm also not sure if something like golang/go#64883 would make it possible for the library to handle this automatically.

@gkampitakis
Copy link
Owner

Hey 👋 That is correct. I can't say it's a bug but rather a feature of supporting multiple calls of matchSnapshot in a single test.

in some (if not all?)

It's all cases.

I have spent some time looking into this, but couldn't find a good solution unfortunately. I would be willing to do a breaking change (some people are using snaps with a count) as the correct way would be do run a test mulitple times and verify with the same snapshot each time but I don't want to remove the ability to be able to call matchSnapshot multiple times in a single test.

The reason why this is difficult to solve is because there is no way to differentiate a snaps.MatchSnapshot is called due to the -count>1 or the snaps.MatchSnapshot is called multiple times inside the test.

As for the TestMain I am pretty it's called only once when you invoke go test and not count times.

To sum up, from the snaps perspective

This

func TestMyTest(t *testing.T) {
  t.Run("my test", func (t *testing.T) {
  	snaps.MatchSnapshot(t, "hello world")
    snaps.MatchSnapshot(t, "hello world")
  }
}

and this can't be differentiated.

// go test -count=2 ./...
func TestMyTest(t *testing.T) {
  t.Run("my test", func (t *testing.T) {
  	snaps.MatchSnapshot(t, "hello world")
  }
}

I can check again my notes and what I tried in the past as it has been some time. As for the go issue it could potentially help in oder to identify if calling with -count>1.

@G-Rath
Copy link
Contributor Author

G-Rath commented Jan 9, 2024

What you've said is pretty much word-for-word what I was thinking too, and why I went looking for golang/go#64883 but then I managed to get myself confused in trying to reproduce it 😅

I've put a comment on that testing.Count issue.

@gkampitakis gkampitakis added the question Further information is requested label Jan 9, 2024
@gkampitakis
Copy link
Owner

Okay found my old notes and spent some time brainstorming with a new solution. I think have something working, but need to spend some time refining it before creating a pr. My solution interfers a bit with clean up functionality and I wouldn't like to introduce a bug there.

@G-Rath
Copy link
Contributor Author

G-Rath commented Jan 14, 2024

Cool - fwiw I was thinking you could also support this by introducing a new method for resetting/clearing the global registry for a test which could be called whenever the test started:

func TestMyTest(t *testing.T) {
	t.Run("my test", func(t *testing.T) {
		snaps.Start(t)
		snaps.MatchSnapshot(t, "hello world")
		snaps.MatchSnapshot(t, "hello world")
	})
}

@gkampitakis
Copy link
Owner

gkampitakis commented Jan 14, 2024

I tried to do something similar indeed without affecting the api but this is a breaking change as some people are already running it with -count>1 but as I said

the correct way would be do run a test mulitple times and verify with the same snapshot each time

Need to run more manual tests to verify I don't introduce any regressions and add unit tests.

Happy to hear your thoughts on this solution.

another-rex pushed a commit to google/osv-scanner that referenced this issue Feb 5, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change | Age | Adoption | Passing |
Confidence |
|---|---|---|---|---|---|---|---|
| [deps.dev/api/v3alpha](https://togithub.com/google/deps.dev) | require
| digest | `00b51ef` -> `c339c64` |
[![age](https://developer.mend.io/api/mc/badges/age/go/deps.dev%2fapi%2fv3alpha/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/deps.dev%2fapi%2fv3alpha/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/deps.dev%2fapi%2fv3alpha/v0.0.0-20240109042716-00b51ef52ece/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/deps.dev%2fapi%2fv3alpha/v0.0.0-20240109042716-00b51ef52ece/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [deps.dev/util/resolve](https://togithub.com/google/deps.dev) |
require | digest | `00b51ef` -> `c339c64` |
[![age](https://developer.mend.io/api/mc/badges/age/go/deps.dev%2futil%2fresolve/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/deps.dev%2futil%2fresolve/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/deps.dev%2futil%2fresolve/v0.0.0-20240109042716-00b51ef52ece/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/deps.dev%2futil%2fresolve/v0.0.0-20240109042716-00b51ef52ece/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [deps.dev/util/semver](https://togithub.com/google/deps.dev) | require
| digest | `1e316b8` -> `c339c64` |
[![age](https://developer.mend.io/api/mc/badges/age/go/deps.dev%2futil%2fsemver/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/deps.dev%2futil%2fsemver/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/deps.dev%2futil%2fsemver/v0.0.0-20240109040450-1e316b822bc4/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/deps.dev%2futil%2fsemver/v0.0.0-20240109040450-1e316b822bc4/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[github.com/gkampitakis/go-snaps](https://togithub.com/gkampitakis/go-snaps)
| require | minor | `v0.4.12` -> `v0.5.2` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgkampitakis%2fgo-snaps/v0.5.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgkampitakis%2fgo-snaps/v0.5.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgkampitakis%2fgo-snaps/v0.4.12/v0.5.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgkampitakis%2fgo-snaps/v0.4.12/v0.5.2?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[github.com/ianlancetaylor/demangle](https://togithub.com/ianlancetaylor/demangle)
| require | digest | `964b1d5` -> `1f824a1` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fianlancetaylor%2fdemangle/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fianlancetaylor%2fdemangle/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fianlancetaylor%2fdemangle/v0.0.0-20240117034632-964b1d53ca6c/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fianlancetaylor%2fdemangle/v0.0.0-20240117034632-964b1d53ca6c/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[github.com/jedib0t/go-pretty/v6](https://togithub.com/jedib0t/go-pretty)
| require | patch | `v6.5.3` -> `v6.5.4` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fjedib0t%2fgo-pretty%2fv6/v6.5.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fjedib0t%2fgo-pretty%2fv6/v6.5.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fjedib0t%2fgo-pretty%2fv6/v6.5.3/v6.5.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fjedib0t%2fgo-pretty%2fv6/v6.5.3/v6.5.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [go](https://go.dev/) ([source](https://togithub.com/golang/go)) |
golang | patch | `1.21.5` -> `1.21.6` |
[![age](https://developer.mend.io/api/mc/badges/age/golang-version/go/1.21.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/golang-version/go/1.21.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/golang-version/go/1.21.5/1.21.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/golang-version/go/1.21.5/1.21.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| golang.org/x/exp | require | digest | `1b97071` -> `2c58cdc` |
[![age](https://developer.mend.io/api/mc/badges/age/go/golang.org%2fx%2fexp/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/golang.org%2fx%2fexp/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/golang.org%2fx%2fexp/v0.0.0-20240119083558-1b970713d09a/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/golang.org%2fx%2fexp/v0.0.0-20240119083558-1b970713d09a/?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [google.golang.org/grpc](https://togithub.com/grpc/grpc-go) | require
| minor | `v1.60.1` -> `v1.61.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/google.golang.org%2fgrpc/v1.61.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/google.golang.org%2fgrpc/v1.61.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/google.golang.org%2fgrpc/v1.60.1/v1.61.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/google.golang.org%2fgrpc/v1.60.1/v1.61.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[google.golang.org/protobuf](https://togithub.com/protocolbuffers/protobuf-go)
| require | minor | `v1.31.0` -> `v1.32.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/google.golang.org%2fprotobuf/v1.32.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/google.golang.org%2fprotobuf/v1.32.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/google.golang.org%2fprotobuf/v1.31.0/v1.32.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/google.golang.org%2fprotobuf/v1.31.0/v1.32.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gkampitakis/go-snaps
(github.com/gkampitakis/go-snaps)</summary>

###
[`v0.5.2`](https://togithub.com/gkampitakis/go-snaps/compare/v0.5.1...v0.5.2)

[Compare
Source](https://togithub.com/gkampitakis/go-snaps/compare/v0.5.1...v0.5.2)

###
[`v0.5.1`](https://togithub.com/gkampitakis/go-snaps/releases/tag/v0.5.1)

[Compare
Source](https://togithub.com/gkampitakis/go-snaps/compare/v0.5.0...v0.5.1)

#### What's Changed

- fix: replace `Print` with `Println` by
[@&#8203;G-Rath](https://togithub.com/G-Rath) in
[gkampitakis/go-snaps#94

**Full Changelog**:
gkampitakis/go-snaps@v0.5.0...v0.5.1

###
[`v0.5.0`](https://togithub.com/gkampitakis/go-snaps/releases/tag/v0.5.0)

[Compare
Source](https://togithub.com/gkampitakis/go-snaps/compare/v0.4.12...v0.5.0)

#### What's Changed

- docs: improve readme code formatting and grammar by
[@&#8203;G-Rath](https://togithub.com/G-Rath) in
[gkampitakis/go-snaps#85
- docs: improve `TestMain` references by
[@&#8203;G-Rath](https://togithub.com/G-Rath) in
[gkampitakis/go-snaps#86
- chore(docs): minor improvements by
[@&#8203;gkampitakis](https://togithub.com/gkampitakis) in
[gkampitakis/go-snaps#89
- chore: clean up test mocks and change getTestID param order by
[@&#8203;gkampitakis](https://togithub.com/gkampitakis) in
[gkampitakis/go-snaps#92
- feat: don't create multiple snapshots when -test.count>1 by
[@&#8203;gkampitakis](https://togithub.com/gkampitakis) in
[gkampitakis/go-snaps#90

#### Breaking changes ❗

On `v0.5.0` when running tests with `test.count>1` flag a call to create
a snapshot will not create multiple instances of the same snapshot, but
it will create the snapshot once and then subsequent execution will test
against that snapshot. Look at issue
[gkampitakis/go-snaps#87

#### New Contributors

- [@&#8203;G-Rath](https://togithub.com/G-Rath) made their first
contribution in
[gkampitakis/go-snaps#85

**Full Changelog**:
gkampitakis/go-snaps@v0.4.12...v0.5.0

</details>

<details>
<summary>jedib0t/go-pretty (github.com/jedib0t/go-pretty/v6)</summary>

###
[`v6.5.4`](https://togithub.com/jedib0t/go-pretty/releases/tag/v6.5.4)

[Compare
Source](https://togithub.com/jedib0t/go-pretty/compare/v6.5.3...v6.5.4)

#### What's Changed

- table: fix SuppressTrailingSpaces removing spaces from the beginning
by [@&#8203;ilya-lesikov](https://togithub.com/ilya-lesikov) in
[jedib0t/go-pretty#295
- table: fix documentation for merges by
[@&#8203;jedib0t](https://togithub.com/jedib0t) in
[jedib0t/go-pretty#296

#### New Contributors

- [@&#8203;ilya-lesikov](https://togithub.com/ilya-lesikov) made their
first contribution in
[jedib0t/go-pretty#295

**Full Changelog**:
jedib0t/go-pretty@v6.5.3...v6.5.4

</details>

<details>
<summary>golang/go (go)</summary>

###
[`v1.21.6`](https://togithub.com/golang/go/compare/go1.21.5...go1.21.6)

</details>

<details>
<summary>grpc/grpc-go (google.golang.org/grpc)</summary>

### [`v1.61.0`](https://togithub.com/grpc/grpc-go/releases/tag/v1.61.0):
Release 1.61.0

[Compare
Source](https://togithub.com/grpc/grpc-go/compare/v1.60.1...v1.61.0)

### New Features

- resolver: provide method, `AuthorityOverrider`, to allow
resolver.Builders to override the default authority for a `ClientConn`.
(EXPERIMENTAL)
([#&#8203;6752](https://togithub.com/grpc/grpc-go/issues/6752))
- Special Thanks:
[@&#8203;Aditya-Sood](https://togithub.com/Aditya-Sood)
- xds: add support for mTLS Credentials in xDS bootstrap ([gRFC
A65](github.com/grpc/proposal/blob/8c31bfedded5f0a51c4933e9e9a8246122f9c41a/A65-xds-mtls-creds-in-bootstrap.md))
([#&#8203;6757](https://togithub.com/grpc/grpc-go/issues/6757))
- Special Thanks: [@&#8203;atollena](https://togithub.com/atollena)
- server: add `grpc.WaitForHandlers` `ServerOption` to cause
`Server.Stop` to block until method handlers return. (EXPERIMENTAL)
([#&#8203;6922](https://togithub.com/grpc/grpc-go/issues/6922))

### Performance Improvements

- grpc: skip compression of empty messages as an optimization
([#&#8203;6842](https://togithub.com/grpc/grpc-go/issues/6842))
    -   Special Thanks: [@&#8203;jroper](https://togithub.com/jroper)
- orca: use atomic pointer to improve performance in server metrics
recorder ([#&#8203;6799](https://togithub.com/grpc/grpc-go/issues/6799))
- Special Thanks:
[@&#8203;danielzhaotongliu](https://togithub.com/danielzhaotongliu)

### Bug Fixes

- client: correctly enable TCP keepalives with OS defaults on windows
([#&#8203;6863](https://togithub.com/grpc/grpc-go/issues/6863))
- Special Thanks: [@&#8203;mmatczuk](https://togithub.com/mmatczuk)
- server: change some stream operations to return `UNAVAILABLE` instead
of `UNKNOWN` when underlying connection is broken
([#&#8203;6891](https://togithub.com/grpc/grpc-go/issues/6891))
- Special Thanks:
[@&#8203;mustafasen81](https://togithub.com/mustafasen81)
- server: fix `GracefulStop` to block until all method handlers return
(v1.60 regression).
([#&#8203;6922](https://togithub.com/grpc/grpc-go/issues/6922))
- server: fix two bugs that could lead to panics at shutdown when using
[`NumStreamWorkers`](https://pkg.go.dev/google.golang.org/grpc#NumStreamWorkers)
(EXPERIMENTAL).
([#&#8203;6856](https://togithub.com/grpc/grpc-go/issues/6856))
- reflection: do not send invalid descriptors to clients for files that
cannot be fully resolved
([#&#8203;6771](https://togithub.com/grpc/grpc-go/issues/6771))
    -   Special Thanks: [@&#8203;jhump](https://togithub.com/jhump)
- xds: don't fail channel/server startup when xds creds is specified,
but bootstrap is missing certificate providers
([#&#8203;6848](https://togithub.com/grpc/grpc-go/issues/6848))
- xds: Atomically read and write xDS security configuration client side
([#&#8203;6796](https://togithub.com/grpc/grpc-go/issues/6796))
- xds/server: fix RDS handling for non-inline route configs
([#&#8203;6915](https://togithub.com/grpc/grpc-go/issues/6915))

</details>

<details>
<summary>protocolbuffers/protobuf-go
(google.golang.org/protobuf)</summary>

###
[`v1.32.0`](https://togithub.com/protocolbuffers/protobuf-go/releases/tag/v1.32.0)

[Compare
Source](https://togithub.com/protocolbuffers/protobuf-go/compare/v1.31.0...v1.32.0)

**Full Changelog**:
protocolbuffers/protobuf-go@v1.31.0...v1.32.0

This release contains commit
protocolbuffers/protobuf-go@bfcd647,
which fixes a denial of service vulnerability by preventing a stack
overflow through a default maximum recursion limit. See
[golang/protobuf#1583
and
[golang/protobuf#1584
for details.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on monday" in timezone
Australia/Sydney, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/google/osv-scanner).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants