Pharo 12 CI - Error: "Instance of Time class did not understand #millisecondsToRun:"

[labordep]

Hi,

I have a bug in all of my CI project tests execution (GitHub and GitLab) on Ubuntu, MacOS and Windows for Pharo 12: Instance of Time class did not understand #millisecondsToRun:

Example here : https://github.com/labordep/Collectable/actions/runs/7513306515/job/20454911062

[Ducasse]

Thanks this is an effect of our massive surgery to remove Time from the bootstrap. This is strange because we were not thinking to remove it. I see in older versions

Time class >> millisecondsToRun: timedBlock
	"Answer the number of milliseconds timedBlock takes to return its value."

	^(self microsecondsToRun: timedBlock) // 1e3

It looks like the Benchmark package is not loaded.

[Ducasse]

No the Benchmark package is loaded. Now I remember that we defined

BlockClosure >> millisecondsToRun
	"Answer the number of milliseconds the receiver takes to return its value."

	^ self microsecondsToRun // 1e3

So we should probably reintroduce Time class >> millisecondsToRun: may be in the bench package. The point is that we do not need Time to do it but this is not true because

BlockClosure >> microsecondsToRun
	"Answer the number of milliseconds the receiver takes to return its value."

	| initialMicroseconds |
	initialMicroseconds := Time microsecondClockValue.
	self value.
	^ Time microsecondClockValue - initialMicroseconds

so we should discuss. The solution is not difficult but this is more a design question. I have the impression that we should reintroduction Time class>>#millisecondsToRun

[jbrichau]

I made a PR for this at SmalltalkCI: https://github.com/hpi-swa/smalltalkCI/pull/625

So, in case you are not planning to re-introduce this, if the PR is accepted this will be fixed.

[Ducasse]

Thanks I was about to fix it but this is good to have the time to think.

[guillep]

Yes, we should reintroduce it and deprecate it. Thanks @jbrichau for updating smalltalkci!

[theseion]

I think something went wrong. The latest commit on the Pharo12 branch is 3ffc5637840a873d5f1a74453d8855a418813a0f but the build version is stuck at a commit from December: 1645336259151e603fddc9b8aeba8ac67378a2f5. When I download a new image, it still contains Time in the Kernel package but some of the methods, such as #millisecondClockValue are missing. That method still exists on the filesystem though at the currenth HEAD: https://github.com/pharo-project/pharo/blob/3ffc5637840a873d5f1a74453d8855a418813a0f/src/System-Time/Time.class.st#L126.

SystemVersion current asString "'Pharo-12.0.0+build.1258.sha.1645336259151e603fddc9b8aeba8ac67378a2f5 (64 Bit)'"

[jbrichau]

@theseion Where do you notice this? Because I just ran a build which requires my latest commit on Pharo 12 and it is using the latest code: https://github.com/SeasideSt/Grease/actions/runs/7533917583/job/20507353031

I also do not see the millisecondsToRun: method in the latest commit: https://github.com/pharo-project/pharo/blob/3ffc5637840a873d5f1a74453d8855a418813a0f/src/System-Time/Time.class.st

[jbrichau]

@theseion Sorry, I did not read your comment entirely. When I download a Pharo 12, I get:

Pharo-12.0.0+SNAPSHOT.build.1295.sha.3ffc5637840a873d5f1a74453d8855a418813a0f (64 Bit)

[theseion]

You're right. The issue appears to be with Pharo Launcher. And that method is gone indeed.

[labordep]

I no longer have this problem on my GitLab and GitHub repositories. Thanks for your fixes!

[Ducasse]

I pushed a fix by reintroducing milliseconds.... and this is good that the smalltalkci got updated too.

[jbrichau]

@Ducasse Will it be deprecated or not? If it will stay, then I will push that change back to SmalltalkCI as it would be an unnecessary difference that is accounted for between platforms. Getting rid of unnecessary things is always good ;-).

[Ducasse]

Guillermo wants to deprecate it. I did not deprecate to avoid a notification that may be break but may be I should.