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

Some tests fail on Squeak #12

Open
fniephaus opened this issue Feb 22, 2016 · 11 comments
Open

Some tests fail on Squeak #12

fniephaus opened this issue Feb 22, 2016 · 11 comments

Comments

@fniephaus
Copy link
Contributor

The following tests fail on Squeak-trunk:

  • STONTests>>testRoomExitCycles (fails)
  • STONWriterTests>>testDictionaryWithComplexKeys (fails)
  • STON.Tests.STONReaderTests>>testUnknow (errors)

It'd be nice if STON was fully Squeak-compatible, because we are using STON in smalltalkCI.

@svenvc
Copy link
Owner

svenvc commented Feb 24, 2016

Hi Fabio,

The idea and ideal is to have STON running on all Smalltalk implementations, and maybe even on other dynamically typed languages. The question is of course who has the man power to do the ports and to maintain them.

Ports to Gemstone and Amber exist and all Pharo versions are supported. So it certainly is doable and I am supportive of such ports (meaning I might make minor changes upstream to facilitate the port).

That being said, I am not able to put real effort into such a port myself. It also might be necessary to do a custom version, as some Pharo specifics might get into the implementation (but not in the specification though).

I just resynced the github repo with the upstream repo http://ss3.gemstone.com/ss/STON (or on StHub, http://www.smalltalkhub.com/mc/SvenVanCaekenberghe/STON/main).

How do you load STON ? Where can I see your failures with backtraces ?

Sven

@fniephaus
Copy link
Contributor Author

smalltalkCI uses STON for the configuration file.
I've copied the STON code into smalltalkCI's repository, primarily to minimize the number of dependencies that need to be loaded from external resources. In order to make all tests pass on Squeak, I took the lazy approach and just removed failing tests.

@fniephaus
Copy link
Contributor Author

Sorry, I don't have backtraces at hand at the moment, since I'm currently working on something else.
I hope to have time to make it better next time I have to touch STON again :)

@svenvc
Copy link
Owner

svenvc commented Nov 20, 2016

Hi Fabio,

Any progress ?

Sven

@fniephaus
Copy link
Contributor Author

Mh no, not really. Sorry about that. I will follow up as soon as I have some time to work on this.

@svenvc
Copy link
Owner

svenvc commented Nov 20, 2016

OK, no problem.

@fniephaus
Copy link
Contributor Author

@svenvc, do you intend to maintain the code on ss3.gemstone.com and here on GitHub?
Currently, the configuration will cause smalltalkCI to load the Ston code from ss3.gemstone.com, so the other packages in repository/ are actually not being used during testing on Travis.
There's for instance a package called STON-Text support which does not seem to be on GitHub?
And just a heads up: in order to support both, Pharo and Squeak, we obviously need dialect-specific packages. I will keep you posted on my progress...

@svenvc
Copy link
Owner

svenvc commented Nov 24, 2016

Yes, for me the ss3.gemstone.com repo comes first.
On each commit, I manually sync with the sthub,com repo.
Only from time to time do I export to git.
Sometimes I make a mistake keeping all these in sync.

STON-Text Support is there to help the Pharo modularization efforts,
I just committed it to git.

First we have to find out why there is a problem on Squeak, then we'll see how we can fix it.
Maybe it is something easy to fix. There are some extension to some system Traits, are there Traits in Squeak?

In any case, thanks for doing this.

@fniephaus
Copy link
Contributor Author

Here are some of the issues I already identified:

  • asDictionary is used in a few tests,
    as: Dictionary would be compatible to Pharo and Squeak AFAIK
  • isHealthy is not available in Squeak
  • String representation of Time does not contain nanoseconds in Squeak
  • Serialization of classes that are not available seems to be broken in Squeak

And there's probably more...

At least, it looks like the SmallDictionary loading error in Squeak is gone :)

@svenvc
Copy link
Owner

svenvc commented Nov 25, 2016

I tried myself in Squeak 4.5

These are the warnings before loading:

This package depends on the following classes:
  TClassDescription
  OrderedDictionary
  SmallDictionary
  TClass
You must resolve these dependencies before you will be able to load these definitions: 
  OrderedDictionary class>>fromSton:
  OrderedDictionary>>stonOn:
  SmallDictionary class>>fromSton:
  SmallDictionary>>stonOn:
  TClass>>stonOn:
  TClassDescription>>stonContainSubObjects

Especially the Traits are important (there is also TApplyingOnClassSide). This how this kind of code need to be written in Pharo. There are only a couple of methods in there, they must be added to the correct classes.

When loading, this is the Transcript output:

STONCStyleCommentsSkipStream>>readInto:startingAt:count: (ZnByteStringBecameWideString is Undeclared) 
STONTests class>>fastReadFromFileNamed: (ZnBufferedReadStream is Undeclared) 
STONTests class>>fastWrite:toFileNamed: (ZnBufferedWriteStream is Undeclared) 

These Zn classes are Pharo specific. The first one needs resolution, the others are not really needed I guess.

I added the following methods (quick&dirty):

Collection>>asDictionary
	^ self as: Dictionary
Dictionary>>isHealthy
	^ true
NotFound>>signalFor: anObject 
	^ (self object: anObject) signal

The #isHealthy is bogus of course.

The FileReference/FileLocator related code and tests should be skipped.

With these I get only 11 errors, #testClasses then fails 5 times.
The rest is related to cycles and the #isHealthy problem.

Summary:

  • write the Trait methods directly on the classes where needed
  • figure out the cycles/isHealthy problem
  • skip non-supported classes

That's it for now

@svenvc
Copy link
Owner

svenvc commented Oct 10, 2018

A quick note: the current implementation no longer uses Traits which should make porting easier.

2d0ab3a

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

Successfully merging a pull request may close this issue.

2 participants