Fix marshalling test (compression issue was fixed in 5.1.1)

[OlivierNicole]

See https://ocaml.org/releases/5.1.1. Currently this test fails with 5.1.1.

[hhugo]

Why is the CI not complaining about this ?

[OlivierNicole]

I’m not sure; maybe its 5.1.1 compiler is configured differently from mine?

[hhugo]

Ah yes, I guess we could condition the test on Compression.compression_supported

[hhugo]

This is the only test checking compression with marshal. I'd prefer to keep it enabled on ocaml > 5.1.0 as it runs and pass in the CI. We could try to come up with a better condition to decide whether compression is enabled though.

[OlivierNicole]

After thinking about it more, I don’t think that is the right decision. The situation at present is the following:

• The test passes on 5.1.0, because 5.1.0 has mandatory zstd integration. But this was changed in 5.1.1 because it caused dependency problems and it has lead to the release of 5.1.1;
• On 5.1.1 and later, the test passes or fail depending on how the OCaml install was configured. What happens is that the condition ocaml_5_1 is true and so the test attempts to unmarshal compressed data. It works if OCaml was built with ZSTD support and compiler-libs is linked in (which is the case here).

But the building of OCaml with ZSTD support happens implicitly, if zlib is available at configure time, which is not something the programmer necessarily thinks about. To wit, on my NixOS zlib is not available unless I explicitly ask for it in my shell, and so my switch was compiled without ZSTD without me even thinking about it. As a consequence, this test has been constantly failing on my machine on all OCaml versions except the (more or less deprecated) 5.1.0, and it took me several hours to find out exactly why. It is nice if all tests pass on the CI but the experience is not very good for us JSOO developers.

For this reason I propose the following: remove this test from the file and move it to a separate test, that will be enabled only on 5.1.1 or later and will test Compression.compression_supported. (I don’t see much worth in testing on 5.1.0.)

What do you think?

[hhugo]

For this reason I propose the following: remove this test from the file and move it to a separate test, that will be enabled only on 5.1.1 or later and will test Compression.compression_supported. (I don’t see much worth in testing on 5.1.0.)

What do you think?

Sounds good

[OlivierNicole]

I pushed this to the branch. Could you re-open the PR?

[hhugo]

Can't re-open it, can you create a new one