Tests are failing on Android

[jcubic]

Tests are failing on Android

[jcubic]

I got a reply from the Zlib author he said that they don't know of any recent change in the output. But he also said that it's not guaranteed that the binary data will always be the same. The only guarantee is that they decompress(compress(x)) == x and that decompress will always be backward compatible. So we can keep the binary blob as a fixture and decompress both the new pack file and fixture. and compare data inside. I only need to check the read data from the pack file using isomorphic-git.

[jcubic]

I'm closing the issue, since the original problem got resolved.

[jcubic]

I think that we need to figure out which service is failing Azure, SauseLabds, or BrowserStack, and ask support about this.

[inverted-capital]

this is fixed in PR #1864

[jcubic]

The real problem is that the zlib compress library was updated in Chrome 120 on Android. And the test compare binary blob of pack file. So the tests need to be update to decompress the data before comparing. This is what Chrome team suggested, to not compare binary compression blobs.

I attempted to fix the tests, but I'm not sure how pack files are created, locally all tests pass but on CI there are even more failing tests.

I run the test with:

npx nps test.setup && npx nps test.jest

[inverted-capital]

Pardon my repetition, but I think it is a bad idea to abandon binary compatibility checks just because a single platform has recently failed a compatibility check that has stood for nearly 5 years, and still stands on all other platforms.

[inverted-capital]

Further, binary compatibility is the only way we can guarantee that objects on all the platforms can interoperate together. The testing combinations are too vast to confirm interop between all the platforms, so binary equality is the simplest way to guarantee this compatibility. This safety should never be abandoned.

[jcubic]

We can still test binary compatibility if the compression algorithm has a stable API. Decompressing old and new stuff gives the same results. We can have a workaround by calling deflate on the packfiles and not worry about comparing binary blobs.

But to fix the tests (simply adding delfate didn't fix the tests) I need to look closer into packfile implementation.

[inverted-capital]

But, surely that would not test the case where a new packfile was created on Android, and then opened on Ubuntu FireFox, for example ? If the file format on android has changed, we cannot be sure it will decompress on all the other platforms, unless we are sure it is binary equal, right ?

[inverted-capital]

That is nice of him to respond so quickly.

zlib (or any other format) cannot be forwards compatible unless it is binary equal.

If compressNew() is running on android with this changed format, and `decompressOld() is running on another platform that is older than the new Android format, then the following is NOT guaranteed:

decompressOld( compressNew( x ) ) == x

I will leave this issue be - hopefully no further issues come up related to it. It is really weird that it changed on Android at all.

[jcubic]

But this is what is tested (but the other way around) There is an old compressed binary that is tested with a new decompress. Zlib author also said that there were no changes in the output, so maybe it's Chrome that changed the binary.

[inverted-capital]

Yes, it is tested the other way around, but that is exactly the problem.

decompressOld( compressNew( x ) ) == x

is not the same test as what you are doing with:

decompressNew( compressOld( x ) ) == x

because decompressNew() is backwards compatible, but decompressOld() is not forwards compatible.

Something is wrong with the Android platform implementation of CompressionStream and the best overall is that we get the Android platform fixed, since this probably subtly affects a lot of other projects. I have an faint idea how to test this on Android so I'll keep thinking about it, and if I can show the Android devs the issue succinctly, I'll cross post back here if you'd like to be kept updated.

[jcubic]

But why do you need this compatibility? Will you move binary blobs between platforms? This is impossible to test even before the change I made.

[inverted-capital]

This would occur in the scenario that someone has a "dumb" git http server, and their webapp generates some commits on Android, and then another client connects from Windows and needs to decompress, but the pack files are in this latest zlib format.

To fail, it would also mean that the zlib part wasn't working, which I think it is, so this isn't really a big problem - I'm just saying we have weakened some of the interop guarantees between platforms that used to exist thanks to binary equality.

Before you relaxed the equality constraints, cross platform interaction was being tested implicitly since binary exactness guarantees that this process works on all platforms even if all platforms interacted with each other.

[inverted-capital]

Also I'm simply answering because you asked the question - I think the decision you've made for this library is a good one and was researched thoroughly.