Skip to content

testPrintAndEvaluate test fails in Pharo 7.0#72

Merged
SergeStinckwich merged 2 commits into
PolyMathOrg:developmentfrom
SergeStinckwich:development
Jul 13, 2018
Merged

testPrintAndEvaluate test fails in Pharo 7.0#72
SergeStinckwich merged 2 commits into
PolyMathOrg:developmentfrom
SergeStinckwich:development

Conversation

@SergeStinckwich

Copy link
Copy Markdown
Member

Fix #71

@WernerK

WernerK commented Jul 12, 2018

Copy link
Copy Markdown
Contributor

Hi Serge
you want somebody to have a look at this, ok:

1.of course you are aware of the fact that you can delete or put into comments the pragma.

2.OpalCompiler >> evaluate: is deprecated in 7.0. for example
Smalltalk compiler evaluate: f storeString
could be changed to something like (dont know whether i get that right freehand, look at OpalCompiler>>evaluate:):
Smalltalk compiler source: f storeString;evaluate

3.you let a var-accessor return something without changing the underlying var, imo a very clever solution that obviously works without problems, but a trick nevertheless.

it probably makes sense if the pharo chiefs do away with pragmas that are not simple to find, but i guess they dont expect everybody and his aunt to find your clever trick. if they have forgotten to add a possibility to change the time for a single test method, perhaps one should inform them of that problem. in a way this change proposes that one should always find a workaround in polymath for problems that will of course regularly occur in a development version of pharo (and after a while can disappear). personally i would think its no problem if tests are red in a development version of pharo, as long as everything is green in the stable pharo. lets say the pharo chiefs add such a possibility, are you sure that you will not forget to adapt ArbitraryPrecisionFloatTest class>>defaultTimeLimit and the rest?

actually thats a reason why i dont like to do anything - polymath or whatever - in pharo7. i run too often into bugs & problems.

  1. the reason that the AppVeyor build runs into problems doesnt seem to be the change in this test method or? the changes are ok and work.
    werner

@WernerK

WernerK commented Jul 12, 2018

Copy link
Copy Markdown
Contributor

if the change is the reason for the difference in the results of appveyor and travis, then they perhaps build different 7.0 images or use different operating systems. in the first case i would just wait & repeat, in the second case, mmh that would be difficult .
werner

@WernerK

WernerK commented Jul 12, 2018

Copy link
Copy Markdown
Contributor

with 2. i made an error, its only on the class side deprecated, hence this is no problem
werner

}

{ #category : #accessing }
ArbitraryPrecisionFloatTest class >> defaultTimeLimit [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this block be inlined as

defaultTimeLimit [ ^ 120 seconds ]

or, again, is what you've done the convention?

@SergeStinckwich

Copy link
Copy Markdown
Member Author

Werner, I don't understand the point 3. Can you try to be actionnable ?

@SergeStinckwich SergeStinckwich merged commit 8ac659a into PolyMathOrg:development Jul 13, 2018
@nicolas-cellier-aka-nice

Copy link
Copy Markdown
Contributor

Why is XXXCompiler evaluate: deprecated ??? Isn't it a usefull API ???
Python, Matlab, ... whatever interpreter HAS an eval function, as we always did.
We definitely MUST have one in Pharo 7.0.

I doubt that replacing by Smalltalk compiler source: f storeString;evaluate is a progress at all!!!
It's an ugly workaround, and I hope not the recommended one for Christ sake!!!

So what is the recommended replacement?
Isn't there an automated rewrite rule for deprecated/renamed stuff?
(I thought this was one of the very nice things that Pharo brought).

@SergeStinckwich

SergeStinckwich commented Jul 13, 2018

Copy link
Copy Markdown
Member Author

I think the reason is that you can have many compilers in your image. In Pharo 6.1, you have the old Compiler and new one OpalCompiler. In Pharo 7.0, old Compiler has been removed and replaced by OpalCompiler.

If you look at my changes, you will see that I replace :
Compiler evaluate: f storeString by Smalltalk compiler evaluate: f storeString.

Smalltalk compiler returns an instance of OpalCompiler in both Pharo 6.1 and 7.0.

AsbtractCompiler class give the API of compilers.

@nicolas-cellier-aka-nice

Copy link
Copy Markdown
Contributor

Ah OK, that's fine then! The message from Werner were not that clear ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants