Setup a framework for provider tests #93
Conversation
Demonstrate usage by writing an initial spec for the Bandcamp provider.
Extraction errors will be thrown at a later stage, if necessary. New behavior is consistent with other providers and the new test spec.
Simple implementation which ignores request body and headers, but so does SnapStorage which we already use.
Now provider tests run without having to pass the `--no-check` flag.
Comment out Bandcamp lookup test, its HTML testdata is too bloated.
I want to reduce the snapshot size by serializing URLs as strings before starting to commit more provider testdata. Shortening of overlong paths of Spotify track API URLs is already working.
Abandon the `URL` type for properties of `HarmonyRelease` in favor of plain strings, which are easier to handle in a number of scenarios: - There can be serialization/deserialization issues with dynamic Fresh components: An URL property may silently turn into a string property when it is rendered on the client-side - It becomes easier to have a JSON API later when everything is JSON already (which URL is not) - No more issues when using `structuredClone` during merging - Smaller test snapshots without redundant URL serializations
Currently they are restored as soon as the scope of the `describe` function has been left, i.e. the tests were registered but not run yet. This only worked in most cases because the stubbed `globalThis.fetch` gets assigned to `MetadataProvider.fetch` which is mostly used, but `globalThis.fetch` is no longer stubbed when the tests actually run! Instead of letting the stubs be restored automatically with `using`, we could do it manually, but that is not really necessary for any test.
Cached test responses now use the same data reduction technique as SnapStorage does in production. It is no longer necessary to stub `fetch` itself during provider tests.
Intentionally using the same test case as for Deezer in order to make the testdata reusable for future merge tests.
Artist and album images which were repeated for each track have been stripped from the testdata API responses.
Making the test context available to the `ReleaseLookupTest.assert` callback gives us the flexibility to have as many snapshot assertions as we want. Explicit assertions are also easier to understand than having an opt-out behavior with the `skipSnapshot` flag.
kellnerd
added
documentation
Pinning the minor version should avoid unexpected deno fmt/lint changes to cause CI failures again in the future.
|
Since you asked in the meeting: I glanced through this and the test structure makes sense to me (without any prior experience looking at the code). :-) |
|
I was able to follow the instructions and run the tests fine on macOS (after upgrading Deno to 2.x and removing the lockfile), and the helpers you've added in providers/test_stubs.ts look...helpful! 🙂 I can't confidently say how quickly I'd be able to write such tests without refreshing my memory of the codebase, but it looks very clean and well-organized overall. |
|
Thank you for the feedback @derat and @mwiencek! I wonder whether you got the same error as GH Actions when it was still running Deno 1.46.3. |
|
Yep, well first I got an error about |
I am opening this PR for two reasons:
Also closes #83.