Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toBeFile in the memos is great, exposes a weakness elsewhere... #221

Closed
nedtwigg opened this issue Feb 14, 2024 · 2 comments
Closed

toBeFile in the memos is great, exposes a weakness elsewhere... #221

nedtwigg opened this issue Feb 14, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@nedtwigg
Copy link
Member

nedtwigg commented Feb 14, 2024

In our memoizing API, we don't have any concept of facets, so it's a bit simpler.

  • StringMemo<T>
    • toMatchDisk(sub: String = "") : T
    • toBe(expected: String): T
  • BinaryMemo<T>
    • toMatchDisk(sub: String = ""): T
    • toBeFile(subpath: String): T

I really like this structure. You always have the toMatchDisk() which puts it into a selfie-managed file that doesn't care whether it's a string or binary. For strings, you can leave selfie's file management behind and store it as an inline String with toBe. And for binary, it makes sense that you might want to put a certain extension on it and view it in a normal file explorer, in which case you say toBeFile and you can specify the correct file extension and all that.

I think toBe for string data is perfect, and toBeFile is the perfect analogy for the binary case. The name might not be perfect.

  • Good parts
    • toBe is perfect, toBeFile piggy backs on that well
    • the File part makes clear that it's not inline, it's at this file
  • Bad parts
    • toBeFile sounds a little bit like toMatchDisk, although it's quite different

Meh. Good enough.

Facets

What we have now in 1.x: assertion selfies can have "facets", which makes them a bit more complex than memo selfies.

fun expectSelfie(actual: String) = expectSelfie(Snapshot.of(actual))
fun expectSelfie(actual: ByteArray) = expectSelfie(Snapshot.of(actual))
fun expectSelfie(actual: Snapshot) = DiskSelfie(actual, deferredDiskStorage)

class DiskSelfie : LiteralStringSelfie {
  fun toMatchDisk(sub: String = ""): DiskSelfie
}
class LiteralStringSelfie {
  fun toBe(expected: String) : LiteralStringSelfie
  fun facet(facet: String) = LiteralStringSelfie(actual, listOf(facet))
}

The good thing about this is that you can do fluent chaining

expectSelfie(complexHtml).toMatchDisk()
  .facet("statusCode").toBe("200")
  .facet("md").toBe_TODO()

The bad thing is that we never really know what type we're going to get.

We could add facetBinary("png").toBeFile("screenshot.png"). The problem is that ideally

  • expectSelfie(actual: ByteArray) should return something which has toMatchDisk and toBeFile
  • once you call facet, you should lose the ability to say toMatchDisk
    • the point of toMatchDisk is to dump it and only look at it again if it changes
    • the point of facets is to pull important parts of those dumps out and do inline assertions with toBe
@nedtwigg nedtwigg added the enhancement New feature or request label Feb 14, 2024
@nedtwigg
Copy link
Member Author

nedtwigg commented Feb 14, 2024

Candidate solution:

interface FluentFacet {
  fun facet(facet: String): StringFacet
  fun facetBinary(facet: String): BinaryFacet
}
interface StringFacet : FluentFacet {
  fun toBe(expected: String): FluentFacet
}
interface BinaryFacet : FluentFacet {
  fun toBeFile(subpath: String): FluentFacet
}
class DiskSelfie<Self : DiskSelfie> {
  fun toMatchDisk(sub: String = ""): Self
}
class StringSelfie : DiskSelfie<StringSelfie>, StringFacet {}
class BinarySelfie : DiskSelfie<BinarySelfie>, BinaryFacet {}

fun expectSelfie(actual: ByteArray) : BinarySelfie
fun expectSelfie(actual: String) : StringSelfie
fun expectSelfie(actual: Snapshot) : StringSelfie // can call facetBinary("").toBeFile() to workaround string bias

@nedtwigg
Copy link
Member Author

nedtwigg commented Feb 14, 2024

I like it. Slight change, kotlin's generics can't handle Self types for now. So

class DiskSelfie<Self : DiskSelfie> {
  fun toMatchDisk(sub: String = ""): Self
}
class StringSelfie : DiskSelfie<StringSelfie>
// turned into
open class DiskSelfie {
  open fun toMatchDisk(sub: String = ""): DiskSelfie
}
class StringSelfie : DiskSelfie {
  override fun toMatchDisk(sub: String): StringSelfie
}

Just as ergonomic to use, and fewer generics is always better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant