automerge / automerge-repo

MIT License
419 stars 43 forks source link

fix test: "throws an error if we try to import an invalid document" #333

Closed HerbCaudill closed 2 months ago

HerbCaudill commented 3 months ago

The VS Code test runner was choking on this test - not sure why, because it's syntactically valid, but upon inspection it did seem like it wasn't intended to be written the way it was. I've fixed it in a way that makes sense to me and the test runner is happy.

before:

expect(() => {
  repo.import<TestDoc>(A.init<TestDoc> as unknown as Uint8Array)
}).toThrow()

after:

expect(() => {
  repo.import<TestDoc>(new Uint8Array([1, 2, 3]))
}).toThrow()
acurrieclark commented 3 months ago

From memory, the test was supposed to check that if a document (rather than a Uint8Array) was passed in, it would throw an error.

Admittedly, passing a document will show a type error, but that was the original plan.

HerbCaudill commented 3 months ago

From memory, the test was supposed to check that if a document (rather than a Uint8Array) was passed in, it would throw an error.

Admittedly, passing a document will show a type error, but that was the original plan.

ah ok. I'll update it.

HerbCaudill commented 3 months ago

hm that actually doesn't throw...

image

HerbCaudill commented 3 months ago

This is the actual behavior:

image

HerbCaudill commented 3 months ago

I've committed these two additional tests, documenting the current behavior of Repo.import({ foo: 123 })and Repo.import(A.from({ foo: 123 })). I'd suggest merging this fix and then deciding whether that's the behavior we want. (My 2 cents is that it's not a problem that it doesn't throw when you give it a parameter of the wrong type. We shouldn't be expected to do input validation in every function in the codebase, that's what TypeScript is for.)