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

test: cleanup duplicate snapshots #902

Closed
wants to merge 3 commits into from
Closed

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Apr 5, 2024

While working on #889 I found go-snaps had a race condition that meant it sometimes duplicates snapshots - this didn't cause any tests to fail, but was confusing and annoying when I was using the snapshots for mass-checking small tweaks to the new output format.

I wrote a small script to clean up these duplicates, but amazingly @gkampitakis has yet again jumped on this quickly and gotten a fix out before I had a chance to mention this to the rest of the team let alone push up my script 😄

This uses my script to cleanup the duplicates and then updates go-snaps to the fixed version so that new test runs won't have this problem.

For those interested, this is the Python script I wrote
#!/usr/bin/env python


import glob
import re


def deduplicate_snapshots(snapshots):
  by_name = {}

  for snapshot in snapshots:
    if snapshot[1] in by_name:
      are_identical = len(snapshot) == len(by_name[snapshot[1]]) and [
        i for i, j in zip(snapshot, by_name[snapshot[1]]) if i == j
      ]

      print(
        "  removed duplicate of",
        snapshot[1].strip(),
        "(were identical)" if are_identical else "",
      )
      continue
    by_name[snapshot[1]] = snapshot
  return by_name.values()


def sort_snapshots(snapshots):
  return sorted(
    snapshots,
    key=lambda lines: [
      int(x) if x.isdigit() else x for x in re.split(r"(\d+)", lines[1])
    ],
  )


def sort_snapshots_in_file(filename):
  snapshots = []
  snapshot = []

  with open(filename, "r") as f:
    for line in f.readlines():
      if line == "---\n":  # marks the start of a new snapshot
        snapshots.append(snapshot)
        snapshot = []
      else:
        snapshot.append(line)
  print("found", len(snapshots), "snapshots in", filename)

  snapshots = deduplicate_snapshots(snapshots)
  snapshots = sort_snapshots(snapshots)

  if filename.endswith("v_test.snap") or filename.endswith("v_test2.snap"):
    return
  with open(filename, "w") as f:
    for snapshot in snapshots:
      f.writelines(snapshot)
      f.writelines("---\n")


for snapshot_file in glob.iglob("**/__snapshots__/*.snap", recursive=True):
  sort_snapshots_in_file(snapshot_file)

Relates to gkampitakis/go-snaps#96

@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.37%. Comparing base (ca67f63) to head (6210659).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #902   +/-   ##
=======================================
  Coverage   63.37%   63.37%           
=======================================
  Files         145      145           
  Lines       11883    11883           
=======================================
  Hits         7531     7531           
  Misses       3885     3885           
  Partials      467      467           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 5, 2024

It seems that my script doesn't give the exact same order as go-snaps though I can't see why - it doesn't seem like the order is alphabetical or in test order...?

This might be a suboptimal thing in go-snaps but I wouldn't call it a bug since it seems to be stable across go-snaps runs (both with existing and regenerating snapshots) which is the source of truth so I've just completely generated the snapshots with duplicates and moved on 🤷

@gkampitakis
Copy link

It seems that my script doesn't give the exact same order as go-snaps though I can't see why - it doesn't seem like the order is alphabetical or in test order...?

This might be a suboptimal thing in go-snaps but I wouldn't call it a bug since it seems to be stable across go-snaps runs (both with existing and regenerating snapshots) which is the source of truth so I've just completely generated the snapshots with duplicates and moved on 🤷

Indeed it uses natural sort.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 5, 2024

Indeed it uses natural sort.

Which I went through some annoying conversations with chatgpt to implement in Python without using a package 😅

But na I think the actual problem is some of the new packages have not actually enabled sorting - I'm doing a new PR to address that

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 5, 2024

This should be landed after #904

another-rex pushed a commit that referenced this pull request Apr 9, 2024
While doing #902 I realised we've got a couple of packages using
snapshots but not cleaning them up afterwards - this changes that; I've
also opened #903 adding a script to check all packages using snapshots
are cleaning them.
another-rex pushed a commit that referenced this pull request Apr 18, 2024
This is sort of a rebase of #902 but I decided to do a new PR because
#898 updated `go-snaps` and #904 sorted (literally) `npmrc_test.snap` so
the only thing left is to remove the old snapshots which is just in
`image_test.snap`

Closes #902
another-rex pushed a commit that referenced this pull request Apr 18, 2024
While doing #902 I realised we've got a couple of packages using
snapshots but not cleaning them up afterwards - this adds a basic script
to check for that, based on the existence of the `__snapshots__`
directory.

Here's what the annotations look like:

<img width="739" alt="image"
src="https://github.com/google/osv-scanner/assets/3151613/b85d4500-6d00-4faa-a8d0-6dc1358b4b80">

Note that because annotations are only shown for _files_, we annotate
the first test file in a directory that doesn't contain
`testmain_test.go` to ensure it is visible.
@G-Rath G-Rath deleted the cleanup-snaps branch April 18, 2024 19:46
josieang pushed a commit to josieang/osv-scanner that referenced this pull request Jun 6, 2024
This is sort of a rebase of google#902 but I decided to do a new PR because
google#898 updated `go-snaps` and google#904 sorted (literally) `npmrc_test.snap` so
the only thing left is to remove the old snapshots which is just in
`image_test.snap`

Closes google#902
josieang pushed a commit to josieang/osv-scanner that referenced this pull request Jun 6, 2024
While doing google#902 I realised we've got a couple of packages using
snapshots but not cleaning them up afterwards - this adds a basic script
to check for that, based on the existence of the `__snapshots__`
directory.

Here's what the annotations look like:

<img width="739" alt="image"
src="https://github.com/google/osv-scanner/assets/3151613/b85d4500-6d00-4faa-a8d0-6dc1358b4b80">

Note that because annotations are only shown for _files_, we annotate
the first test file in a directory that doesn't contain
`testmain_test.go` to ensure it is visible.
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.

4 participants