[Seaside-dev] Grease on Pharo7: streams

[Johan Brichau]

Hi all,

There have been a number of changes to the ReadWriteStream implementation in Pharo 7 that break Grease stream tests: https://travis-ci.org/SeasideSt/Grease/jobs/406766720 <https://travis-ci.org/SeasideSt/Grease/jobs/406766720>
I think there is both an issue in Grease for Pharo and in Pharo 7 itself.

* The most important issue is that the following expression returns a different result on Pharo 7 when compared with earlier versions:
	
	(ReadWriteStream on: (String new: 4096)) contents

On Pharo 6, this returns an empty string.
On Pharo 7, this returns a string with 4096 white space chars.

I’m inclined to say that Pharo 7 behavior is correct.
I read through the ANSI document and `contents` should return the future sequences as well… which effectively means returning all those empty chars.
I’m a bit in doubt why this has been different all that time… hence my email.

The core of the issue is that GRPharoPlatform>>readWriteCharacterStream is `ReadWriteStream on: (String new: 4096)`, hence a String instance with a lot of spaces ;)
In GRGemStonePlatform, the readWriteStream is initialized on an empty string.
In GRSqueakPlatform, the RWBinaryOrTextStream class is used on ‘ByteArray new: 4096’.

Changing the implementation of GRPharoPlatform>>readWriteCharacterStream to `ReadWriteStream on: String new` fixes most of the tests.

* The next issue is that `ReadWriteStream>>contents` has been changed in https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04 <https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04>
This means that:

| stream |
stream := GRPlatform current readWriteCharacterStream.
stream
	nextPutAll: 'abc';
	reset.
self assert: stream contents = 'abc’.

This is no longer true in Pharo 7.
ANSI docs says that `reset` should only reset the pointer position and thus the future sequences on the stream should still be returned after resetting. 
Reverting the change in the commit, also fixes that test.
It’s unclear why that change was made though, so I’ll try to talk to someone at Pharo.

Would be nice to hear other’s thoughts.

Cheers
Johan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/seaside-dev/attachments/20180723/4ecda9c3/attachment.html>