Tools: When tests are renamed it doesn't update the snapshots correctly

[Kelbie]

If you rename a test it keeps the old snapshot and appends the new snapshot to the end of the file. Since the tests still pass I suspect that Rome already has some stale snapshots from this bug.

Steps To Reproduce

1. Open up any *.test.ts file for example noVar.test.ts

2. Rename the test from disallow var to disallow var2

3. Run rome test --update-snapshots

4. Look at the snapshot file noVar.test.md which will have the snapshot from the previous name and the new one under the new name.

Expected Results

Snapshot file should only contain new test and not the previously named test.

[yassere]

I poked around a bit, and here's what I think is happening, but please correct me if I'm wrong.

All the snapshot tests within a single test file correspond to a single snapshot file.

When snapshot files are loaded, all the parsed nodes are used to populate an entries Map on a snapshot object that represents the whole file. When a renamed (or new) snapshot test can't find a matching entry in that test file's snapshot, it adds a new entry to the Map but the snapshot object is still marked as snapshot.used = true;

Later, unused snapshot files are deleted (if there are no diagnostics) but there doesn't appear to be any logic for pruning unused entries from a used snapshot. Everything in the entries Map is included when the snapshot is rebuilt and written to file.

[sebmck]

Yeah this isn't really a bug, just a missing feature. All changes will just be made to SnapshotManager. To resolve this:

• Add a used property to SnapshotEntry
• In SnapshotManager#loadSnapshot add used: false on all the entry objects
• In SnapshotManager#set add used: true on the entries object
• In SnapshotManager#buildSnapshot add if (!entry.used && !this.hasFocusedTests) { continue; } to the testNameToEntries population loop. this.hasFocusedTests indicates whether we have a focused test, this is the result of doing something like using test.only. If we ignored unused snapshots then we'd be removing snapshots from the temporarily disabled tests.

[sebmck]

Let me know if anyone wants to do this. Just reply to this issue so multiple people aren't working on it. Just run ./scripts/dev-rome ci --fix after those changes to refresh all the snapshots.

[yassere]

I can work on this.