Change Details

T3365 Snapshot mgmt in Cashtab has been an issue for some time. We initially had an automated approach where - `npm test` ran automatically on `git commit` - This would fail if snapshots required updating - Separately, a `posttest` script added `@generated` to all snapshot files This was abandoned because - Requiring tests to pass before a commit was extremely annoying as a dev, since sometimes you just wanted to save your work - The precommit hook ran Cashtab tests when other repos changed - Another npm dependency was required - Most importantly, the issue wasn't really solved since sometimes `npm test` would miss failing snapshots Back on the "no system" system for some time after this, we still have issues. I think that most of the issues are being caused by `npm test` occasionally missing some unit tests that have impacted snapshots, as `npm test` only runs unit tests from affected files. The CI runs all the tests, occasionally catching a missed snapshot, and throwing an error. Remove the logic that runs a limited set of Cashtab tests locally so that locally running the tests will always run all the unit tests. Add logic so that snapshots are automatically updated when `npm test` is run locally. Remove the `@generated` -- the ones left in Cashtab are from the legacy script, still in files that have not changed since this automation was removed. Snapshots should be reviewed to make sure their update was consistent with the code change, per jest best practices https://jestjs.io/docs/snapshot-testing#best-practices So -- it is still possible for a dev to push a diff up that has failing snapshots. However, this also means the dev did not run `npm test` to check his work before pushing the diff up, which is the kind of thing CI is expected to catch.

T3365 Snapshot mgmt in Cashtab has been an issue for some time. We initially had an automated approach where - `npm test` ran automatically on `git commit` - This would fail if snapshots required updating - Separately, a `posttest` script added `@generated` to all snapshot files This was abandoned because - Requiring tests to pass before a commit was extremely annoying as a dev, since sometimes you just wanted to save your work - The precommit hook ran Cashtab tests when other repos changed - Another npm dependency was required - Most importantly, the issue wasn't really solved since sometimes `npm test` would miss failing snapshots Back on the "no system" system for some time after this, we still have issues. I think that most of the issues are being caused by `npm test` occasionally missing some unit tests that have impacted snapshots, as `npm test` only runs unit tests from affected files using boilerplate logic. This logic has been observed to miss failing tests in the past. The CI runs all the tests, occasionally catching a missed snapshot, and throwing an error. Remove the logic that runs a limited set of Cashtab tests locally so that locally running the tests will always run all the unit tests. Add logic so that snapshots are automatically updated when `npm test` is run locally. Remove the `@generated` -- the ones left in Cashtab are from the legacy script, still in files that have not changed since this automation was removed. Snapshots should be reviewed to make sure their update was consistent with the code change, per jest best practices https://jestjs.io/docs/snapshot-testing#best-practices So -- it is still possible for a dev to push a diff up that has failing snapshots. However, this also means the dev did not run `npm test` to check his work before pushing the diff up, which is the kind of thing CI is expected to catch.

T3365 Snapshot mgmt in Cashtab has been an issue for some time. We initially had an automated approach where - `npm test` ran automatically on `git commit` - This would fail if snapshots required updating - Separately, a `posttest` script added `@generated` to all snapshot files This was abandoned because - Requiring tests to pass before a commit was extremely annoying as a dev, since sometimes you just wanted to save your work - The precommit hook ran Cashtab tests when other repos changed - Another npm dependency was required - Most importantly, the issue wasn't really solved since sometimes `npm test` would miss failing snapshots Back on the "no system" system for some time after this, we still have issues. I think that most of the issues are being caused by `npm test` occasionally missing some unit tests that have impacted snapshots, as `npm test` only runs unit tests from affected files using boilerplate logic. This logic has been observed to miss failing tests in the past. The CI runs all the tests, occasionally catching a missed snapshot, and throwing an error. Remove the logic that runs a limited set of Cashtab tests locally so that locally running the tests will always run all the unit tests. Add logic so that snapshots are automatically updated when `npm test` is run locally. Remove the `@generated` -- the ones left in Cashtab are from the legacy script, still in files that have not changed since this automation was removed. Snapshots should be reviewed to make sure their update was consistent with the code change, per jest best practices https://jestjs.io/docs/snapshot-testing#best-practices So -- it is still possible for a dev to push a diff up that has failing snapshots. However, this also means the dev did not run `npm test` to check his work before pushing the diff up, which is the kind of thing CI is expected to catch.