Running tests on 2.0 and first impression

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Running tests on 2.0 and first impression

Nicolas Cellier
First thing, congrats to Pharo community, I observe a good trend.
It may seem dumb, but it's great to have pharo executable and pharo icons.
Once the image launched, my general feel is good too, though I'm not
fan of multiple keys shortcuts.
It stressed me to the point that I didn't realize the single key
shortcuts were still active in text editor (desperately seeking cmd b+
something... when cmd + shift + N would just browse ivar references).
I guess I'll get used to it.
Browsing the class Object seems a bit long, I guess there's time
wasted scanning the class hierarchy...

Of course, there's a few things to polish.
First I ran the tests, and here is what I have to say about it
- there were two interactions requests, IMO, there shouldn't be any (a
pop up for my name, and a menu)
- there are some failures (10) and errors (3), is this expected? (see
attachment)
- more annoyingly, during the test, the progressbar feedback
disappeared (sporadically flashing) and the TestRunner feedback too
(number of succeded test updates went scarce)

When I opened a FileBrowser to browse the filedOut TestRunner report,
I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
that's surprising.

There I stopped exploring the features and inquired a bit about the failures.
Most failures are related to DateAndTime. I guess they occured because
I was playing with image around midnight with a +01:00 offset.
Early debugging let me see some highly questionable code, using
negative seconds in internal representation is a recipe for trouble..
Having julianDayNumber the instance variable being different from
julianDayNumber the message is also asking for trouble.

Well, just wanted to share this first feedback, i think i have lot of
things to discover in this image (I guess good things, but you know
me, I generally report the bad first).

cheers

Nicolas

tests_Pharo2.0_2013-03-19_23-55-38.txt (824 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Marcus Denker-4

On Mar 20, 2013, at 12:54 AM, Nicolas Cellier <[hidden email]> wrote:

> First thing, congrats to Pharo community, I observe a good trend.
> It may seem dumb, but it's great to have pharo executable and pharo icons.
> Once the image launched, my general feel is good too, though I'm not
> fan of multiple keys shortcuts.
> It stressed me to the point that I didn't realize the single key
> shortcuts were still active in text editor (desperately seeking cmd b+
> something... when cmd + shift + N would just browse ivar references).
> I guess I'll get used to it.
> Browsing the class Object seems a bit long, I guess there's time
> wasted scanning the class hierarchy...
>
> Of course, there's a few things to polish.
> First I ran the tests, and here is what I have to say about it
> - there were two interactions requests, IMO, there shouldn't be any (a
> pop up for my name, and a menu)

How to not require a name when a tests does something that needs it?
When running tests on the build server it sets a name before and removes
it later, but I don't think that the test runner should do that by default.

> - there are some failures (10) and errors (3), is this expected? (see
> attachment)

-> There are some tests that fail when running like this that do not fail
on the build server. We should put issues for them on the tracker and fix them.

-> There seems to be a strange Time related bug happening only around midnight.
Yes, someone should have a look a that, too.
(First step is to submit an issue).

> - more annoyingly, during the test, the progressbar feedback
> disappeared (sporadically flashing) and the TestRunner feedback too
> (number of succeded test updates went scarce)
>
ProgressBar still needs to be improved. I think there is even an issue for that.

> When I opened a FileBrowser to browse the filedOut TestRunner report,
> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
> that's surprising.
>
FileBrowser is not used a lot and there are of course no UI level tests.
So there will be bugs.

> There I stopped exploring the features and inquired a bit about the failures.
> Most failures are related to DateAndTime. I guess they occured because
> I was playing with image around midnight with a +01:00 offset.

Yes, two people already saw this bug happening, we should add an issue to the tracker.

In general it just shows: Any issue that is not fixed by someone is not fixed. ;-)


        Marcus





Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Sven Van Caekenberghe-2
Just out of curiosity I ran all tests using a #20593 image and the latest Mac OS X pharo vm on my machine.

9749 run, 9686 passes, 2 skipped, 58 expected failures, 3 failures, 2 errors, 0 unexpected passes

Failures:
ReleaseTest>>#testUndeclared
SimulateKeystrokesSpecification>>#testSimulateKeystroke
SimulateKeystrokesSpecification>>#testSimulateKeystrokes

Errors:
MCStReaderTest>>#testCommentWithStyle
MCStReaderTest>>#testMethodWithStyle

And the fact that Undeclared was not empty was due to extra loaded code of my own.
 
Like Marcus said, I already reported the time problem close to midnight:

https://pharo.fogbugz.com/f/cases/7602
https://code.google.com/p/pharo/issues/detail?id=7643

The other remarks are valid of course.

Sven

On 20 Mar 2013, at 08:51, Marcus Denker <[hidden email]> wrote:

> On Mar 20, 2013, at 12:54 AM, Nicolas Cellier <[hidden email]> wrote:
>
>> First thing, congrats to Pharo community, I observe a good trend.
>> It may seem dumb, but it's great to have pharo executable and pharo icons.
>> Once the image launched, my general feel is good too, though I'm not
>> fan of multiple keys shortcuts.
>> It stressed me to the point that I didn't realize the single key
>> shortcuts were still active in text editor (desperately seeking cmd b+
>> something... when cmd + shift + N would just browse ivar references).
>> I guess I'll get used to it.
>> Browsing the class Object seems a bit long, I guess there's time
>> wasted scanning the class hierarchy...
>>
>> Of course, there's a few things to polish.
>> First I ran the tests, and here is what I have to say about it
>> - there were two interactions requests, IMO, there shouldn't be any (a
>> pop up for my name, and a menu)
>
> How to not require a name when a tests does something that needs it?
> When running tests on the build server it sets a name before and removes
> it later, but I don't think that the test runner should do that by default.
>
>> - there are some failures (10) and errors (3), is this expected? (see
>> attachment)
>
> -> There are some tests that fail when running like this that do not fail
> on the build server. We should put issues for them on the tracker and fix them.
>
> -> There seems to be a strange Time related bug happening only around midnight.
> Yes, someone should have a look a that, too.
> (First step is to submit an issue).
>
>> - more annoyingly, during the test, the progressbar feedback
>> disappeared (sporadically flashing) and the TestRunner feedback too
>> (number of succeded test updates went scarce)
>>
> ProgressBar still needs to be improved. I think there is even an issue for that.
>
>> When I opened a FileBrowser to browse the filedOut TestRunner report,
>> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
>> that's surprising.
>>
> FileBrowser is not used a lot and there are of course no UI level tests.
> So there will be bugs.
>
>> There I stopped exploring the features and inquired a bit about the failures.
>> Most failures are related to DateAndTime. I guess they occured because
>> I was playing with image around midnight with a +01:00 offset.
>
> Yes, two people already saw this bug happening, we should add an issue to the tracker.
>
> In general it just shows: Any issue that is not fixed by someone is not fixed. ;-)
>
>
> Marcus
>
>
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Marcus Denker-4

On Mar 20, 2013, at 9:57 AM, Sven Van Caekenberghe <[hidden email]> wrote:

> Just out of curiosity I ran all tests using a #20593 image and the latest Mac OS X pharo vm on my machine.
>

I added Issues for those two:

> SimulateKeystrokesSpecification>>#testSimulateKeystroke
> SimulateKeystrokesSpecification>>#testSimulateKeystrokes
>
https://pharo.fogbugz.com/f/cases/10066/SimulateKeystrokesSpecification-test-failing


> Errors:
> MCStReaderTest>>#testCommentWithStyle
> MCStReaderTest>>#testMethodWithStyle
>
https://pharo.fogbugz.com/f/cases/10065/MCStReaderTest-failing-tests-when-run-with-TestRunner
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Nicolas Cellier
In reply to this post by Marcus Denker-4
2013/3/20 Marcus Denker <[hidden email]>:

>
> On Mar 20, 2013, at 12:54 AM, Nicolas Cellier <[hidden email]> wrote:
>
>> First thing, congrats to Pharo community, I observe a good trend.
>> It may seem dumb, but it's great to have pharo executable and pharo icons.
>> Once the image launched, my general feel is good too, though I'm not
>> fan of multiple keys shortcuts.
>> It stressed me to the point that I didn't realize the single key
>> shortcuts were still active in text editor (desperately seeking cmd b+
>> something... when cmd + shift + N would just browse ivar references).
>> I guess I'll get used to it.
>> Browsing the class Object seems a bit long, I guess there's time
>> wasted scanning the class hierarchy...
>>
>> Of course, there's a few things to polish.
>> First I ran the tests, and here is what I have to say about it
>> - there were two interactions requests, IMO, there shouldn't be any (a
>> pop up for my name, and a menu)
>
> How to not require a name when a tests does something that needs it?
> When running tests on the build server it sets a name before and removes
> it later, but I don't think that the test runner should do that by default.
>
>> - there are some failures (10) and errors (3), is this expected? (see
>> attachment)
>
> -> There are some tests that fail when running like this that do not fail
> on the build server. We should put issues for them on the tracker and fix them.
>
> -> There seems to be a strange Time related bug happening only around midnight.
> Yes, someone should have a look a that, too.
> (First step is to submit an issue).
>
>> - more annoyingly, during the test, the progressbar feedback
>> disappeared (sporadically flashing) and the TestRunner feedback too
>> (number of succeded test updates went scarce)
>>
> ProgressBar still needs to be improved. I think there is even an issue for that.
>
>> When I opened a FileBrowser to browse the filedOut TestRunner report,
>> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
>> that's surprising.
>>
> FileBrowser is not used a lot and there are of course no UI level tests.
> So there will be bugs.
>
>> There I stopped exploring the features and inquired a bit about the failures.
>> Most failures are related to DateAndTime. I guess they occured because
>> I was playing with image around midnight with a +01:00 offset.
>
> Yes, two people already saw this bug happening, we should add an issue to the tracker.
>
> In general it just shows: Any issue that is not fixed by someone is not fixed. ;-)
>
>
>         Marcus
>
>

Totally agree, but for this specific bug I think that  the process can
be improved by increased code reviews.
The fact that a negative seconds inst var and a julianDayNumber inst
var different from julianDayNumber message was chosen indicates that
current implementation is a quick hack, a workaround for solving some
problems, but clearly not a long term solution. IMO it creates as many
problems as it solves.
Diff has been added lately, and it is a real progress for reviewing
code, but the effort to participate is still a bit high. More eyes,
more reviews...

The fact that bug only show up around midnight is true only for
Europeans which have small offsets from UTC, I guess the probability
of bug shall increase in California or Japan.

Nicolas

>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Sean P. DeNigris
Administrator
In reply to this post by Marcus Denker-4
Marcus Denker-4 wrote
How to not require a name when a tests does something that needs it?
When running tests on the build server it sets a name before and removes
it later, but I don't think that the test runner should do that by default.
MCWorkingCopyTest already temporarily sets the author name, why not do the same with the test Nicolas is pointing out?

OT... this smells to me like another example of the value of a "skipped test" concept in SUnit... if the information is supplied, no problem, if the dialog is cancelled, or the image is headless, skip the test.
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Frank Shearar-3
On 20 March 2013 11:56, Sean P. DeNigris <[hidden email]> wrote:
> Marcus Denker-4 wrote
>> How to not require a name when a tests does something that needs it?
>> When running tests on the build server it sets a name before and removes
>> it later, but I don't think that the test runner should do that by
>> default.
>
> MCWorkingCopyTest already temporarily sets the author name, why not do the
> same with the test Nicolas is pointing out?

Or perhaps the test runner can do this: one risks forgetting that some
action triggers the dialog, and not notice in one's dev image. (And of
course the test runner can _unset_ the author if it had to fake an
author.)

frank

> OT... this smells to me like another example of the value of a "skipped
> test" concept in SUnit... if the information is supplied, no problem, if the
> dialog is cancelled, or the image is headless, skip the test.
>
>
>
> -----
> Cheers,
> Sean
> --
> View this message in context: http://forum.world.st/Running-tests-on-2-0-and-first-impression-tp4677439p4677504.html
> Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Camillo Bruni-3
In reply to this post by Nicolas Cellier
>>
> Totally agree, but for this specific bug I think that  the process can
> be improved by increased code reviews.
> The fact that a negative seconds inst var and a julianDayNumber inst
> var different from julianDayNumber message was chosen indicates that
> current implementation is a quick hack, a workaround for solving some
> problems, but clearly not a long term solution. IMO it creates as many
> problems as it solves.

That is a totally different discussion and most probably the tests are
wrong, as with most of the date and time stuff from squeak. The dimensions
of this mess are hard to grasp, but using a reference point expressed in
local time is a very bad thing. So we changed that and all internal
representations of DateAndTime are in UTC. All external representations
of DateAndTime are in local time.

That implies that whatever you inspect and see is in UTC, negative
offsets make perfectly sense if your time-zone lags behind UTC. I wish
I had time to fully clear up the mess, and add all basic accessors for
utc.


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen

On Mar 20, 2013, at 1:20 PM, Camillo Bruni wrote:

>>>
>> Totally agree, but for this specific bug I think that  the process can
>> be improved by increased code reviews.
>> The fact that a negative seconds inst var and a julianDayNumber inst
>> var different from julianDayNumber message was chosen indicates that
>> current implementation is a quick hack, a workaround for solving some
>> problems, but clearly not a long term solution. IMO it creates as many
>> problems as it solves.
>
> That is a totally different discussion and most probably the tests are
> wrong, as with most of the date and time stuff from squeak. The dimensions
> of this mess are hard to grasp, but using a reference point expressed in
> local time is a very bad thing. So we changed that and all internal
> representations of DateAndTime are in UTC. All external representations
> of DateAndTime are in local time.
>
> That implies that whatever you inspect and see is in UTC, negative
> offsets make perfectly sense if your time-zone lags behind UTC. I wish
> I had time to fully clear up the mess, and add all basic accessors for
> utc.
>
>
Nicolas wasn't talking of offset.

When I (in Norwegian timezone) do
DateAndTime midnight

I get an instance with instvars:
seconds  -3600
offset: 1 hour duration.

If it were indeed supposed to return local time instvar, I would expect an instance with
julianDayNumber one less, and seconds 23 *3600.

Cheers,
Henry
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen

On Mar 20, 2013, at 2:01 PM, Henrik Johansen wrote:

>
>
> If it were indeed supposed to return local time instvar, I would expect an instance with
Bah, forgot to proofread, meant "If it were indeed supposed to return midnight local time, I would expect a DateAndTime with UTC instance variables where"
> julianDayNumber one less, and seconds 23 *3600.
>
> Cheers,
> Henry



Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Sven Van Caekenberghe-2
In reply to this post by Henrik Sperre Johansen

On 20 Mar 2013, at 14:01, Henrik Johansen <[hidden email]> wrote:

> On Mar 20, 2013, at 1:20 PM, Camillo Bruni wrote:
>
>>> Totally agree, but for this specific bug I think that  the process can
>>> be improved by increased code reviews.
>>> The fact that a negative seconds inst var and a julianDayNumber inst
>>> var different from julianDayNumber message was chosen indicates that
>>> current implementation is a quick hack, a workaround for solving some
>>> problems, but clearly not a long term solution. IMO it creates as many
>>> problems as it solves.
>>
>> That is a totally different discussion and most probably the tests are
>> wrong, as with most of the date and time stuff from squeak. The dimensions
>> of this mess are hard to grasp, but using a reference point expressed in
>> local time is a very bad thing. So we changed that and all internal
>> representations of DateAndTime are in UTC. All external representations
>> of DateAndTime are in local time.
>>
>> That implies that whatever you inspect and see is in UTC, negative
>> offsets make perfectly sense if your time-zone lags behind UTC. I wish
>> I had time to fully clear up the mess, and add all basic accessors for
>> utc.
>>
> Nicolas wasn't talking of offset.
>
> When I (in Norwegian timezone) do
> DateAndTime midnight
>
> I get an instance with instvars:
> seconds  -3600
> offset: 1 hour duration.
>
> If it were indeed supposed to return local time instvar, I would expect an instance with
> julianDayNumber one less, and seconds 23 *3600.
>
> Cheers,
> Henry


Maybe renaming the instance variables to reflect their UTC nature would help, but there is a difference between internal representation and external behaviour - if it is documented that seems OK to me.

The #midnight example probably needs an extra normalisation of the internal representation, which would indeed shift it to what you would expect.

Constructive criticism is always welcome, but these changes to DateAndTime were made many months ago - with great effort. Yes, there is still work to be done, and although it is maybe logical, it is a bit dissappointing to hear it right after a release.

Pharo is a continuous improvement process.

Sven

--
Sven Van Caekenberghe
http://stfx.eu
Smalltalk is the Red Pill


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Camillo Bruni-3
In reply to this post by Henrik Sperre Johansen

On 2013-03-20, at 14:01, Henrik Johansen <[hidden email]> wrote:

> On Mar 20, 2013, at 1:20 PM, Camillo Bruni wrote:
>>> Totally agree, but for this specific bug I think that  the process can
>>> be improved by increased code reviews.
>>> The fact that a negative seconds inst var and a julianDayNumber inst
>>> var different from julianDayNumber message was chosen indicates that
>>> current implementation is a quick hack, a workaround for solving some
>>> problems, but clearly not a long term solution. IMO it creates as many
>>> problems as it solves.
>>
>> That is a totally different discussion and most probably the tests are
>> wrong, as with most of the date and time stuff from squeak. The dimensions
>> of this mess are hard to grasp, but using a reference point expressed in
>> local time is a very bad thing. So we changed that and all internal
>> representations of DateAndTime are in UTC. All external representations
>> of DateAndTime are in local time.
>>
>> That implies that whatever you inspect and see is in UTC, negative
>> offsets make perfectly sense if your time-zone lags behind UTC. I wish
>> I had time to fully clear up the mess, and add all basic accessors for
>> utc.
>>
>>
> Nicolas wasn't talking of offset.
>
> When I (in Norwegian timezone) do
> DateAndTime midnight
>
> I get an instance with instvars:
> seconds  -3600
> offset: 1 hour duration.
>
> If it were indeed supposed to return local time instvar, I would expect an instance with
> julianDayNumber one less, and seconds 23 *3600.

right, so it looks like DateAndTime>>#year:month:day:hour:minute:second:nanoSecond:offset: does not take the seconds nor the offset into account when calculating the julian day number.

I opened a bug entry with an intermediate fix for this:
https://pharo.fogbugz.com/default.asp?10069
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Miguel Moquillon
In reply to this post by Nicolas Cellier
You're lucky, because in my own machine running GNU/Linux Ubuntu 12.04
the tests running in Pharo 2.0 either kills the VM or most of the time
freezes the environment, depending on the weather...

Miguel

Le 20/03/2013 00:54, Nicolas Cellier a écrit :

> First thing, congrats to Pharo community, I observe a good trend.
> It may seem dumb, but it's great to have pharo executable and pharo icons.
> Once the image launched, my general feel is good too, though I'm not
> fan of multiple keys shortcuts.
> It stressed me to the point that I didn't realize the single key
> shortcuts were still active in text editor (desperately seeking cmd b+
> something... when cmd + shift + N would just browse ivar references).
> I guess I'll get used to it.
> Browsing the class Object seems a bit long, I guess there's time
> wasted scanning the class hierarchy...
>
> Of course, there's a few things to polish.
> First I ran the tests, and here is what I have to say about it
> - there were two interactions requests, IMO, there shouldn't be any (a
> pop up for my name, and a menu)
> - there are some failures (10) and errors (3), is this expected? (see
> attachment)
> - more annoyingly, during the test, the progressbar feedback
> disappeared (sporadically flashing) and the TestRunner feedback too
> (number of succeded test updates went scarce)
>
> When I opened a FileBrowser to browse the filedOut TestRunner report,
> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
> that's surprising.
>
> There I stopped exploring the features and inquired a bit about the failures.
> Most failures are related to DateAndTime. I guess they occured because
> I was playing with image around midnight with a +01:00 offset.
> Early debugging let me see some highly questionable code, using
> negative seconds in internal representation is a recipe for trouble..
> Having julianDayNumber the instance variable being different from
> julianDayNumber the message is also asking for trouble.
>
> Well, just wanted to share this first feedback, i think i have lot of
> things to discover in this image (I guess good things, but you know
> me, I generally report the bad first).
>
> cheers
>
> Nicolas

--
L'Intelligence Artificielle n'a aucune chance face à la Stupidité Naturelle


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

EstebanLM
which vm, when it freezes, etc.

a little bit more of info could help on fixing bugs :)

Esteban

On Mar 20, 2013, at 2:34 PM, Miguel Moquillon <[hidden email]> wrote:

> You're lucky, because in my own machine running GNU/Linux Ubuntu 12.04 the tests running in Pharo 2.0 either kills the VM or most of the time freezes the environment, depending on the weather...
>
> Miguel
>
> Le 20/03/2013 00:54, Nicolas Cellier a écrit :
>> First thing, congrats to Pharo community, I observe a good trend.
>> It may seem dumb, but it's great to have pharo executable and pharo icons.
>> Once the image launched, my general feel is good too, though I'm not
>> fan of multiple keys shortcuts.
>> It stressed me to the point that I didn't realize the single key
>> shortcuts were still active in text editor (desperately seeking cmd b+
>> something... when cmd + shift + N would just browse ivar references).
>> I guess I'll get used to it.
>> Browsing the class Object seems a bit long, I guess there's time
>> wasted scanning the class hierarchy...
>>
>> Of course, there's a few things to polish.
>> First I ran the tests, and here is what I have to say about it
>> - there were two interactions requests, IMO, there shouldn't be any (a
>> pop up for my name, and a menu)
>> - there are some failures (10) and errors (3), is this expected? (see
>> attachment)
>> - more annoyingly, during the test, the progressbar feedback
>> disappeared (sporadically flashing) and the TestRunner feedback too
>> (number of succeded test updates went scarce)
>>
>> When I opened a FileBrowser to browse the filedOut TestRunner report,
>> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
>> that's surprising.
>>
>> There I stopped exploring the features and inquired a bit about the failures.
>> Most failures are related to DateAndTime. I guess they occured because
>> I was playing with image around midnight with a +01:00 offset.
>> Early debugging let me see some highly questionable code, using
>> negative seconds in internal representation is a recipe for trouble..
>> Having julianDayNumber the instance variable being different from
>> julianDayNumber the message is also asking for trouble.
>>
>> Well, just wanted to share this first feedback, i think i have lot of
>> things to discover in this image (I guess good things, but you know
>> me, I generally report the bad first).
>>
>> cheers
>>
>> Nicolas
>
> --
> L'Intelligence Artificielle n'a aucune chance face à la Stupidité Naturelle
>
>


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen
In reply to this post by Sven Van Caekenberghe-2

On Mar 20, 2013, at 2:21 PM, Sven Van Caekenberghe wrote:

>
> On 20 Mar 2013, at 14:01, Henrik Johansen <[hidden email]> wrote:
>
>> On Mar 20, 2013, at 1:20 PM, Camillo Bruni wrote:
>>
>>>> Totally agree, but for this specific bug I think that  the process can
>>>> be improved by increased code reviews.
>>>> The fact that a negative seconds inst var and a julianDayNumber inst
>>>> var different from julianDayNumber message was chosen indicates that
>>>> current implementation is a quick hack, a workaround for solving some
>>>> problems, but clearly not a long term solution. IMO it creates as many
>>>> problems as it solves.
>>>
>>> That is a totally different discussion and most probably the tests are
>>> wrong, as with most of the date and time stuff from squeak. The dimensions
>>> of this mess are hard to grasp, but using a reference point expressed in
>>> local time is a very bad thing. So we changed that and all internal
>>> representations of DateAndTime are in UTC. All external representations
>>> of DateAndTime are in local time.
>>>
>>> That implies that whatever you inspect and see is in UTC, negative
>>> offsets make perfectly sense if your time-zone lags behind UTC. I wish
>>> I had time to fully clear up the mess, and add all basic accessors for
>>> utc.
>>>
>> Nicolas wasn't talking of offset.
>>
>> When I (in Norwegian timezone) do
>> DateAndTime midnight
>>
>> I get an instance with instvars:
>> seconds  -3600
>> offset: 1 hour duration.
>>
>> If it were indeed supposed to return local time instvar, I would expect an instance with
>> julianDayNumber one less, and seconds 23 *3600.
>>
>> Cheers,
>> Henry
>
>
> Maybe renaming the instance variables to reflect their UTC nature would help, but there is a difference between internal representation and external behaviour - if it is documented that seems OK to me.
>
> The #midnight example probably needs an extra normalisation of the internal representation, which would indeed shift it to what you would expect.
>
Yeah, seems to be in the resolver of the whole family of year:* constructors.
Attached a fix, as I have no idea how to do the new issue tracking, and couldn't find a tutorial on the website.

> Constructive criticism is always welcome, but these changes to DateAndTime were made many months ago - with great effort. Yes, there is still work to be done, and although it is maybe logical, it is a bit dissappointing to hear it right after a release.
>
> Pharo is a continuous improvement process.

For those involved on a daily basis, yes.
Feedback from others after a major release shouldn't be seen as a source of disappointment. ;)

Cheers,
Henry

DateAndTime class-yearmonthdayhourminutesecondnanoSecondoffset.st (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen

On Mar 20, 2013, at 2:52 PM, Henrik Johansen wrote:

>>
> Yeah, seems to be in the resolver of the whole family of year:* constructors.
> Attached a fix, as I have no idea how to do the new issue tracking, and couldn't find a tutorial on the website.

> <DateAndTime class-yearmonthdayhourminutesecondnanoSecondoffset.st>
Scratch that, definitely not my day today :)

Cheers,
Henry
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Miguel Moquillon
In reply to this post by EstebanLM
Le 20/03/2013 14:36, Esteban Lorenzano a écrit :
> which vm, when it freezes, etc.
>
> a little bit more of info could help on fixing bugs :)
In fact it is the VM provided by the Pharo 2.0 one-click distribution.
Some info below:
$ ./pharo -version
3.9-7 #1 Wed Mar 13 18:22:44 CET 2013 gcc 4.4.3
NBCoInterpreter NativeBoost-CogPlugin-EstebanLorenzano.18 uuid:
a53445f9-c0c0-4015-97a3-be7db8d9ed6b Mar 13 2013
NBCogit NativeBoost-CogPlugin-EstebanLorenzano.18 uuid:
a53445f9-c0c0-4015-97a3-be7db8d9ed6b Mar 13 2013
git://gitorious.org/cogvm/blessed.git Commit:
412abef33cbed05cf1d75329e451d71c0c6aa5a7 Date: 2013-03-13 17:48:50 +0100
By: Esteban Lorenzano <[hidden email]> Jenkins build #14535
Linux linux-ubuntu-10 2.6.32-38-server #83-Ubuntu SMP Wed Jan 4 11:26:59
UTC 2012 x86_64 GNU/Linux
plugin path:
/home/mmoquillon/Applications/Pharo2.0-one-click.app/Contents/Linux/
[default:
/home/mmoquillon/Applications/Pharo2.0-one-click.app/Contents/Linux/]

Miguel

>
> Esteban
>
> On Mar 20, 2013, at 2:34 PM, Miguel Moquillon <[hidden email]> wrote:
>
>> You're lucky, because in my own machine running GNU/Linux Ubuntu 12.04 the tests running in Pharo 2.0 either kills the VM or most of the time freezes the environment, depending on the weather...
>>
>> Miguel
>>
>> Le 20/03/2013 00:54, Nicolas Cellier a écrit :
>>> First thing, congrats to Pharo community, I observe a good trend.
>>> It may seem dumb, but it's great to have pharo executable and pharo icons.
>>> Once the image launched, my general feel is good too, though I'm not
>>> fan of multiple keys shortcuts.
>>> It stressed me to the point that I didn't realize the single key
>>> shortcuts were still active in text editor (desperately seeking cmd b+
>>> something... when cmd + shift + N would just browse ivar references).
>>> I guess I'll get used to it.
>>> Browsing the class Object seems a bit long, I guess there's time
>>> wasted scanning the class hierarchy...
>>>
>>> Of course, there's a few things to polish.
>>> First I ran the tests, and here is what I have to say about it
>>> - there were two interactions requests, IMO, there shouldn't be any (a
>>> pop up for my name, and a menu)
>>> - there are some failures (10) and errors (3), is this expected? (see
>>> attachment)
>>> - more annoyingly, during the test, the progressbar feedback
>>> disappeared (sporadically flashing) and the TestRunner feedback too
>>> (number of succeded test updates went scarce)
>>>
>>> When I opened a FileBrowser to browse the filedOut TestRunner report,
>>> I typed *.txt as a filter and got a RegexSyntaxError nullable closure,
>>> that's surprising.
>>>
>>> There I stopped exploring the features and inquired a bit about the failures.
>>> Most failures are related to DateAndTime. I guess they occured because
>>> I was playing with image around midnight with a +01:00 offset.
>>> Early debugging let me see some highly questionable code, using
>>> negative seconds in internal representation is a recipe for trouble..
>>> Having julianDayNumber the instance variable being different from
>>> julianDayNumber the message is also asking for trouble.
>>>
>>> Well, just wanted to share this first feedback, i think i have lot of
>>> things to discover in this image (I guess good things, but you know
>>> me, I generally report the bad first).
>>>
>>> cheers
>>>
>>> Nicolas
>> --
>> L'Intelligence Artificielle n'a aucune chance face à la Stupidité Naturelle
>>
>>
>


--
L'Intelligence Artificielle n'a aucune chance face à la Stupidité Naturelle


Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen
In reply to this post by Henrik Sperre Johansen

On Mar 20, 2013, at 3:05 PM, Henrik Johansen wrote:

>
> On Mar 20, 2013, at 2:52 PM, Henrik Johansen wrote:
>
>>>
>> Yeah, seems to be in the resolver of the whole family of year:* constructors.
>> Attached a fix, as I have no idea how to do the new issue tracking, and couldn't find a tutorial on the website.
>
>> <DateAndTime class-yearmonthdayhourminutesecondnanoSecondoffset.st>
> Scratch that, definitely not my day today :)
>
> Cheers,
> Henry

Assuming the instvars are indeed now meant to hold UTC-values, here's some more thorough feedback (related to issues occurring around midnight):

- Time initialization uses seconds from primSecondsClock (which returns local time) to set DaysSinceEpoch, with a constructor that expects UTC (in other words, it sets julianDayNumber directly).
I couldn't find any accessors in the image, but I think there's new UTC clock primitives available?

- todayAtMilliSeconds: / todayAtNanoSeconds: use this cached value without doing any checks, so unless someone calls
milliSecondsSinceMidnight (which does the correct checking), they will return false results regardless when clock passes over.

Incorrect direct uses of julianDayNumber instvar:
- #hasEqualTicks: compares julianDayNumber directly vs other instances julianDayNumber, rather than julianDayNumberUTC.
- same for #<

Cheers,
Henry
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Camillo Bruni-3

On 2013-03-20, at 15:42, Henrik Johansen <[hidden email]> wrote:

>
> On Mar 20, 2013, at 3:05 PM, Henrik Johansen wrote:
>
>>
>> On Mar 20, 2013, at 2:52 PM, Henrik Johansen wrote:
>>
>>>>
>>> Yeah, seems to be in the resolver of the whole family of year:* constructors.
>>> Attached a fix, as I have no idea how to do the new issue tracking, and couldn't find a tutorial on the website.
>>
>>> <DateAndTime class-yearmonthdayhourminutesecondnanoSecondoffset.st>
>> Scratch that, definitely not my day today :)
>>
>> Cheers,
>> Henry
>
> Assuming the instvars are indeed now meant to hold UTC-values, here's some more thorough feedback (related to issues occurring around midnight):
>
> - Time initialization uses seconds from primSecondsClock (which returns local time) to set DaysSinceEpoch, with a constructor that expects UTC (in other words, it sets julianDayNumber directly).
> I couldn't find any accessors in the image, but I think there's new UTC clock primitives available?

apparently there is, but I only realized that shortly before the 2.0 release, so we decided to patch some methods instead (#HACK)

> - todayAtMilliSeconds: / todayAtNanoSeconds: use this cached value without doing any checks, so unless someone calls
> milliSecondsSinceMidnight (which does the correct checking), they will return false results regardless when clock passes over.

meh meh meh :D

> Incorrect direct uses of julianDayNumber instvar:
> - #hasEqualTicks: compares julianDayNumber directly vs other instances julianDayNumber, rather than julianDayNumberUTC.
> - same for #<

For comparison it should always use the internal UTC values,
thanks anyways :) I will check that later...

Could you open new issues on pharo.fogbugz.com? I added you as a user.
Reply | Threaded
Open this post in threaded view
|

Re: Running tests on 2.0 and first impression

Henrik Sperre Johansen

On Mar 20, 2013, at 5:24 PM, Camillo Bruni wrote:

Could you open new issues on pharo.fogbugz.com? I added you as a user.

Added 10072-10075 for the things I spotted that might be in need of some extra love.

Cheers,
Henry
12