Skip to content

Conversation

@mbostock
Copy link
Member

@mbostock mbostock commented Aug 9, 2024

This rewrites the build tests so that the hashes for anything under _observablehq aren’t dependent on the actual content. This avoids needing to regenerate the test snapshots whenever we change Framework’s client code (e.g., #1564).

@mbostock mbostock requested a review from Fil August 9, 2024 15:18
@Fil
Copy link
Contributor

Fil commented Aug 9, 2024

Nice. This doesn't seem to cover some of the esm files? (e.g., test/output/build/files/_npm/d3-dsv@3.0.1/cd372fb8.js)

@mbostock
Copy link
Member Author

mbostock commented Aug 9, 2024

This doesn't seem to cover some of the esm files?

What do you mean by “doesn’t cover”? Those files are generated and covered by the test. The caveat is that we don’t actually talk to jsDelivr; we use undici to mock the request, which returns an empty file. That’s in the mockJsDelivr function.

@mbostock
Copy link
Member Author

mbostock commented Aug 9, 2024

I’m only trying to normalize against Framework’s client code changing since that causes churn in the test snapshots. The _npm content won’t cause churn because we already mock the jsDelivr API and hence control when it changes.

@Fil
Copy link
Contributor

Fil commented Aug 9, 2024

I meant they still had a 8 letter hash. (But since it's the hash of empty, it doesn't matter.)

@mbostock
Copy link
Member Author

mbostock commented Aug 9, 2024

Oh, yeah… though now I’m curious why it isn’t e3b0c442 which is the empty hash? I’ll investigate before merging.

@mbostock
Copy link
Member Author

mbostock commented Aug 9, 2024

Ah, okay so it’s the hash of the empty hash. 😂

createHash("sha256").update("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855").digest("hex") // cd372fb85148700fa88095e3492d3f9f5beb43e555e5ff26d95f5a6adc36f8e6

@mbostock mbostock merged commit 152fd4e into main Aug 9, 2024
@mbostock mbostock deleted the mbostock/test-normalize-observable-hash branch August 9, 2024 15:49
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.

3 participants