deno_std icon indicating copy to clipboard operation
deno_std copied to clipboard

`assertEquals` could return `Promise` for `Blob` comparison

Open yuhr opened this issue 4 months ago • 10 comments

Is your feature request related to a problem? Please describe.

Documentation of assertEquals says:

Note: When comparing Blob objects, you should first convert them to Uint8Array using the Blob.bytes() method and then compare their contents.

But what if one wants to compare whole objects that must contain Blobs as property values in a quite deep level of nesting?

Describe the solution you'd like

assertEquals should be able to return Promise<void> to defer such comparison.

In https://github.com/denoland/std/issues/6202#issuecomment-2498053184, @kt3k said that changing assertEquals to async would be a large impact, but it seems unclear how large; the current return type of assertEquals is just void, so no one is supposed to use the return value. And of course non-async part of assertion can still be done synchronously before returning a Promise. Users who don't use Blobs wouldn't have to await assertEquals.

Describe alternatives you've considered

To write assertEquals for every single leaf property of the objects in concern, which is catastrophically painful.

yuhr avatar Jul 26 '25 01:07 yuhr

FWIW, there's currently no special casing for Blobs, the reason it currently throws (under Deno) is due to synchronously checking all props, and a hidden property Symbol(Parts) can have identical parts with different IDs:

import { assertEquals } from 'jsr:@std/assert'

const PARTS = Object.getOwnPropertySymbols(new Blob()).find((s) => s.description === 'Parts')

new Blob()[PARTS] // []
// doesn't throw
assertEquals(new Blob()[PARTS], new Blob()[PARTS])
// doesn't throw
assertEquals(new Blob(), new Blob())

const b1 = new Blob(['a'])
const b2 = new Blob(['a'])

b1[PARTS] // e.g. [ BlobReference { _id: "c42c94b2-809c-4918-afbd-4b2aa3e6c5c3", size: 1 } ]
b2[PARTS] // e.g. [ BlobReference { _id: "bf511325-9af5-4268-a3c0-ad42d41922d0", size: 1 } ]

// // throws
// assertEquals(b1[PARTS], b2[PARTS])
// // throws
// assertEquals(b1, b2)

delete b1[PARTS]
delete b2[PARTS]

// doesn't throw
assertEquals(b1, b2)

The last assertion still doesn't throw even if b2 is new Blob(['b']), but it does if b2 is new Blob(['ab']) (because then the size prop differs).

lionel-rowe avatar Jul 27 '25 09:07 lionel-rowe

To write assertEquals for every single leaf property of the objects in concern, which is catastrophically painful.

I’d recommend writing a serializer for your object first, then comparing the result with the expected output.

kt3k avatar Jul 29 '25 05:07 kt3k

writing a serializer for your object

That sounds like an overengineering. Such a serializer must be reliably lossless and requires another test suite to verify the reliability. Way more painful for general developers.

yuhr avatar Jul 29 '25 07:07 yuhr

I agree that it doesn't make much sense to special-case Blob here — it quickly becomes complicated when you start adding special cases for arbitrary built-in classes, even before adding asynchronicity to the mix. It's not even obvious what a Blob should be converted to — call its bytes method? What about if it's a plain text type, should you call text method instead? Then wrap it in... something? If wrapping in a plain object, should that be counted as equal to a plain object with the same props? What about media types, and file names for File? What about user-defined derived classes of Blob that have additional properties? For that matter, why not special-case Response or other promise-based APIs too?

What could be useful for your use case is a recursive async conversion function (not necessarily a serializer per se), something like this:

async function deepConvertAsync(data: unknown, callback: (x: unknown) => unknown | Promise<unknown>, _seen = new WeakSet()): Promise<unknown> {
	if (_seen.has(data as WeakKey)) return { $$circular: true, data };
	try { _seen.add(data as WeakKey); } catch {/* ignore if invalid WeakKey */}

	if (data != null && typeof data === "object") {
		if (Array.isArray(data)) {
			const arr = Promise.all(data.map((item) => deepConvertAsync(item, callback, _seen)));
			return arr;
		}
		if ([null, Object.prototype].includes(Object.getPrototypeOf(data))) {
			// is a plain object
			const entries = await Promise.all(
				Object.entries(data).map(async ([key, value]) => [key, await deepConvertAsync(value, callback, _seen)]),
			);
			const obj = Object.fromEntries(entries);
			return obj;
		}
		return callback(data);
	}
	return callback(data);
}

Then you decide how the blobs etc. should be converted by passing your own callback:

async function converter(x: unknown): Promise<unknown> {
    if (x instanceof Blob) {
        return { $$blob: await (x.type.split('/', 1)[0] === 'text' ? x.text() : x.bytes()) }
    }
    return x;
}

Usage:

const left = {
    a: [new Blob(['foo'], { type: 'text/plain' })],
    b: { c: new Blob(['bar']) },
}

const right = {
    a: [new Blob(['baz'], { type: 'text/plain' })],
    b: { c: new Blob(['baz']) },
}

assertEquals(
    await deepConvertAsync(left, converter),
    await deepConvertAsync(right, converter),
)

Result:

    [Diff] Actual / Expected

    {
      a: [
        {
-         "$$blob": "foo",
+         "$$blob": "baz",
        },
      ],
      b: {
        c: {
          "$$blob": Uint8Array(3) [
            98,
            97,
-           114,
+           122,
          ],
        },
      },
    }

lionel-rowe avatar Jul 29 '25 10:07 lionel-rowe

It's not even obvious what a Blob should be converted to

Don't overthink it. Blob is just an immutable byte array + MIME type by definition. Comparing the bytes and the type should be sufficient.

What about media types, and file names for File? What about user-defined derived classes of Blob that have additional properties?

why not special-case Response or other promise-based APIs too?

Returning Promise opens up the possibility to do it. The only reason why I specifically picked Blob for this topic is just it's mentioned in the documentation comment. I understand it would be going complicated, but ideally all standardized built-ins should be compared in a sane default way, or at least its comparison method should be overrideable for those who don't like the default way.

What could be useful for your use case is a recursive async conversion function (not necessarily a serializer per se), something like this:

Thanks for the code, but then why not make assertEquals have a built-in escape hatch like the deepConvertAsync? Returning Promise only when a callback is given won't impact existing users.

To be clear, I don't think we should neccessarily extend assertEquals. Instead providing a dedicated API to perform asynchronous assertion with a name like assertEqualsAsync would also be viable. I just mean any generic functionality required only to perform tests should be a part of the testing framework's side, not users' side, as much as possible.

yuhr avatar Jul 29 '25 13:07 yuhr

Don't overthink it. Blob is just an immutable byte array + MIME type by definition. Comparing the bytes and the type should be sufficient.

By that definition, because File extends Blob:

await assertEqualsAsync(new File([], 'a.txt'), new File([], 'b.txt')) // passes assertion despite different file names

why not make assertEquals have a built-in escape hatch like the deepConvertAsync? Returning Promise only when a callback is given won't impact existing users.

Because explicit is generally better than implicit, and returning a promise would mean either changing the return type to Promise<void>, in which case lint tools are liable to emit errors if it's not awaited, or leaving it as void, in which case no warning will be emitted even when failing to await it is a bug.

Instead providing a dedicated API to perform asynchronous assertion with a name like assertEqualsAsync

That's a more promising direction, but the obvious thing for such an API to do would just be assertEquals(await left, await right), which wouldn't help for your nested example, nor would it help when a specific method (e.g. bytes()) needs to be called on certain leaf nodes. An API like deepConvertAsync (whether or not a part of std) is more explicit, more composable, and only adds a minimal amount of complexity to calling code.

lionel-rowe avatar Jul 29 '25 14:07 lionel-rowe

Don't overthink it. Blob is just an immutable byte array + MIME type by definition. Comparing the bytes and the type should be sufficient.

By that definition, because File extends Blob:

The quoted sentence of mine is talking about Blobs. We can cover standard subclasses with another default comparator.

That said, I understand all the default is implicit, and I agree that “explicit is generally better than implicit”.

and returning a promise would mean either changing the return type to

I mean something like this:

declare const assertEquals: <Options extends string | { converter?: Function } | undefined>(
	actual: unknown,
	expected: unknown,
	options?: Options,
) => Options extends { converter: Function } ? Promise<void> : void

declare const converter: Function

assertEquals(0, 0) // void
assertEquals(0, 0, { converter }) // Promise<void>

An API like deepConvertAsync (whether or not a part of std) is more explicit, more composable, and only adds a minimal amount of complexity to calling code.

Hm, now I'm convinced that we should steer this way, and thus this issue is now about supporting such a common pattern in std (adding async variant of mapValues?).

yuhr avatar Jul 29 '25 15:07 yuhr

Hm, now I'm convinced that we should steer this way, and thus this issue is now about supporting such a common pattern in std (adding async variant of mapValues?).

Looks like mapValues is shallow, so it would probably need to be deepMapValues + deepMapValuesAsync (+ possibly shallow mapValuesAsync too?) Not sure if the deep mapped types could be made improved over unknown for everything. There's also mapKeys, but I can't imagine there's much utility for deepMapkeys or deepMapKeysAsync...?

Also mapValuesAsync with identity callback x => x would be equivalent to Promise.properties/Promise.allObject/Promise.hash API as discussed here: Modify Promise.all() to accept an Object as a parameter - ES Discuss

lionel-rowe avatar Jul 30 '25 02:07 lionel-rowe

assertEquals(0, 0, { converter }) // Promise<void>

What does converter do? Is it called for each node of object, or called only for the entire value? What's an example of 'converter' supporting Blob case?

kt3k avatar Jul 30 '25 03:07 kt3k

Hmm that could actually work type-wise. But I don't think supplying converter option should automatically make it Promise<void>, only if converter returns a promise (edit: wait does this make sense? Is it possible to do reliably at runtime? Especially when considering native Promise vs bespoke thenables?)

type AssertEqualsOptions<T, O> = {
    converter?: (x: T) => O
}

declare function assertEquals<T, O>(
    a: T,
    b: T,
    options?: AssertEqualsOptions<T, O>
): O extends Promise<unknown> ? Promise<void> : void

// void
assertEquals(1, 2)

// void
assertEquals(1, 2, {})

// void
assertEquals(1, 2, { converter: (x) => x })

// void | Promise<void>
assertEquals(1, 2, { converter: () => ({} as any) })

// Promise<void>
assertEquals(1, 2, { converter: async (x) => x })

converter here for the nested blob case would be something like deepConvertAsync above with its own converter callback.

async function blobConverter(x: unknown): Promise<unknown> {
    if (x instanceof Blob) return { $$blob: await (x.type.split('/', 1)[0] === 'text' ? x.text() : x.bytes()) }
    return x;
}

await assertEquals(objWithBlobs1, objWithBlobs2, { converter: (x) => deepConvertAsync(x, { converter: blobConverter }) })

lionel-rowe avatar Jul 30 '25 04:07 lionel-rowe