[squeak-dev] Skipping tests (was: [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518))

Marcel Taeumel marcel.taeumel at hpi.de
Mon Apr 27 06:44:07 UTC 2020


I don't think we need an extra status; not even sure how to measure "inconclusive". The traffic light in Squeak's SUnit has already yellow as an extra color compared to related unit testing implementations. Well, I do like to be able to distinguish between assertion failures and other errors at a glance. However, red always feels like bugs that are easier to fix. That's paradoxical, isn't it?

Best,
Marcel
Am 27.04.2020 00:14:51 schrieb Chris Muller <asqueaker at gmail.com>:
On Sun, Apr 26, 2020 at 12:35 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de [mailto:Christoph.Thiede at student.hpi.uni-potsdam.de]> wrote:

> overall status universalis (e.g., red, yellow, green)

I think this list is not yet exhaustive, I would like to add a fourth status (grey) for single tests that are inconclusive.

-1.  Red, yellow, green is universally understood.  The colors are supposed to denote the action required, nothing more.

    Green=none, yellow=investigate, red=action required.

"Inconclusive" would be yellow.  Errors and Failures should both be red, IMO.

If there were a gray, it should mean nothing other than "Off" or "no status available".  Maybe that's what you meant..?

 - Chris

 

Jakob, thanks for the inspirations! How about the following implementation of suitability in SUnit?

MyToolTest >> #testUIStarts

    <suitable: #isMorphic>
    tool := MyTool open.
    self assert: tool isMorph.
    ...

MyToolTest >> #isMorphic
    ^ Project current isMorphic

And maybe class-wise, too:

MyToolTest >> #isSuitable
    ^ self isCI not

MyToolTest >> #isCI
    ^ (Smalltalk classNamed: #SmalltalkCI)
        ifNil: [false]
        ifNotNil: [:ci | (ci environmentNamed: 'CI_STATUS') = 'true' "pseudo"]

For other scenarios where suitability is not obvious, we could use an inconclusive message like:

ToolBuilder findDefault = MorphicToolBuilder
    ifFalse: [self notSuitable: 'Cannot perform this test on custom tool builder, and am too lazy to mock it'].

Or maybe like this?

self
    assertCircumstance: [ToolBuilder findDefault = MorphicToolBuilder]
    description: 'Cannot perform this test on custom tool builder, and am too lazy to mock it'.

And if we had the #suitable: pragma, we could also use this notation for skipping a test:

MyToolTest >> #testLoadsDatabase
    <suitable: false "note by programmer: i don't want to run this atm">
    self doExpensiveAssertion...

Best,
Christoph


Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Chris Muller <asqueaker at gmail.com [mailto:asqueaker at gmail.com]>
Gesendet: Dienstag, 21. April 2020 00:53 Uhr
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Skipping tests (was: [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518))
 
When I find myself swimming in "solutions", I like to take a step back and refocus on the problem for guidance.  What am I _really_ wanting, and why?  For me, it's about

   #1 -- a meaningful, overall status universalis (e.g., red, yellow, green)
   #2 -- ability to defer broken tests until later so their individual status won't affect the overall output of #1
        (2b -- see / edit / run the list of deferred tests)
   #3 -- "notification" of when a deferred test is fixed, but still in the deferred list


It really boils down to making the "overall status" useful by deciding which tests I want to decouple from it, combined with helping me keep that list maintained in the similar way as it informs me about broken tests -- by flagging those tests in some way ("unexpected success" or whatever -- I care a lot less about what words we use in the selectors as much as the ability to meet the above requirements).

 - Chris

On Mon, Apr 20, 2020 at 1:22 PM Jakob Reschke <forums.jakob at resfarm.de [mailto:forums.jakob at resfarm.de]> wrote:

JUnit has assumptions for the "not suitable" case:  https://junit.org/junit5/docs/5.0.0/api/org/junit/jupiter/api/Assumptions.html [https://junit.org/junit5/docs/5.0.0/api/org/junit/jupiter/api/Assumptions.html]
That would match "platform-specific circumstance" and TestInconclusive [https://github.com/hpi-swa-teaching/SpreadSheetTool/tree/master/packages/SpreadSheetTool-Tests.package/TestInconclusive.class].

Then there is the other case to "ignore", "skip", or "disable" a certain test for the time being, for whatever reason. https://junit.org/junit5/docs/current/api/org.junit.jupiter.api/org/junit/jupiter/api/Disabled.html [https://junit.org/junit5/docs/current/api/org.junit.jupiter.api/org/junit/jupiter/api/Disabled.html]

What about the other suggestion to use pragmas? I'd like that (more than listing selectors in another method). But I think it is only practical for the "disable" case because otherwise you would have to put the conditions for "suitable" in an expression in the pragma and would have to evaluate that in the runner ...

Am Mo., 20. Apr. 2020 um 17:09 Uhr schrieb Marcel Taeumel <marcel.taeumel at hpi.de [mailto:marcel.taeumel at hpi.de]>:

Hmm... could be another can-o-worms. :-) Skipping is not the same as an expected failure. Could as well be a platform-specific circumstance that makes a test not suitable.

Best,
Marcel
Am 18.04.2020 15:28:33 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de [mailto:christoph.thiede at student.hpi.uni-potsdam.de]>:
By the way, this also reminds me of some previous ideas about skipping tests in SUnit.

SqueakSheet contains an implementation of TestInconclusive [https://github.com/hpi-swa-teaching/SpreadSheetTool/tree/master/packages/SpreadSheetTool-Tests.package/TestInconclusive.class] and a <skip> pragma [https://github.com/hpi-swa-teaching/SpreadSheetTool/blob/master/packages/SpreadSheetTool-Tests.package/TestCase.extension/class/wantsToTest..st].
Leon chose a similar approach in the PragmaTestCase [https://github.com/MrModder/PragmaTestCase] package.

Maybe we should consider adding something similar to the Trunk? Personally, I find it more convenient to tag expected failures or "test that take too much time for the moment" directly at the relevant test case, not in an extra #expectedFailures method.

Best,
Christoph


Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Thiede, Christoph
Gesendet: Samstag, 18. April 2020 15:22 Uhr
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518)
 
Thanks for your arguments! Yeah, we should really take CI more seriously, a red bar shouldn't be something you see every day.

> Note that we're already excluding some test cases:
> https://github.com/squeak-smalltalk/squeak-app/blob/b063518c61711ab7a0ca89ffa3d2494925fac823/smalltalk-ci/Squeak64-trunk.ston#L8-L12 [https://github.com/squeak-smalltalk/squeak-app/blob/b063518c61711ab7a0ca89ffa3d2494925fac823/smalltalk-ci/Squeak64-trunk.ston#L8-L12]

Hm, for this purpose of "expected failures, don't run them at all" I think it could be helpful if smalltalkCI printed these tests into the output log.
Example:

#########################
# 4 tests did not pass: #
#########################

SocketTest
 ✗ #testSocketReuse (215ms)

 ✗ #testUDP (10007ms)
...

#########################
# 4 tests were skipped: #
#########################

AllocationTest
 ⛔ #testOneGigAllocation
 ⛔ #testOneMegAllocation
 ⛔ #testOutOfMemorySignal

...

What do you think? :-)

Best,
Christoph

Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Fabio Niephaus <lists at fniephaus.com [mailto:lists at fniephaus.com]>
Gesendet: Freitag, 17. April 2020 17:58 Uhr
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518)
 
Note that we're already excluding some test cases:
https://github.com/squeak-smalltalk/squeak-app/blob/b063518c61711ab7a0ca89ffa3d2494925fac823/smalltalk-ci/Squeak64-trunk.ston#L8-L12 [https://github.com/squeak-smalltalk/squeak-app/blob/b063518c61711ab7a0ca89ffa3d2494925fac823/smalltalk-ci/Squeak64-trunk.ston#L8-L12]


On Fri, Apr 17, 2020 at 5:57 PM Fabio Niephaus <lists at fniephaus.com [mailto:lists at fniephaus.com]> wrote:

On Fri, Apr 17, 2020 at 5:53 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de [mailto:Christoph.Thiede at student.hpi.uni-potsdam.de]> wrote:

> Hmmm... in any case, we want to new bundles to appear on files.squeak.org/trunk [http://files.squeak.org/trunk] right?

Of course, but that does not mean that we have to handle the build as successful, does it? We can also upload the new bundles and then let the build fail.
Or we could fix (or ignore) the four failing tests and move on:

[image.png]

 
Fabio

Best,
Christoph
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Taeumel, Marcel
Gesendet: Freitag, 17. April 2020 17:27:29
An: gettimothy via Squeak-dev
Betreff: Re: [squeak-dev] [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518)
 
Hmmm... in any case, we want to new bundles to appear on files.squeak.org/trunk [http://files.squeak.org/trunk] right? That's why it is called "ALPHA". :-)

Best,
Marcel
Am 17.04.2020 17:22:16 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de [mailto:christoph.thiede at student.hpi.uni-potsdam.de]>:
Sorry for the imprecise question. What I find a bit confusing is the following: Not all tests are passing, but the Travis job still ends up with exit code 0 so we get the mail "build was fixed", and the badge of the repo becomes green. Wouldn't it be more intuitive if the mails sent to the list would keep reporting "still failing" until we have a green bar in the Trunk?

This is the relevant place in the code (in the function check_test_status):
squeak-app/prepare_image.sh [https://github.com/squeak-smalltalk/squeak-app/blob/b063518c61711ab7a0ca89ffa3d2494925fac823/prepare_image.sh#L90-L94]
Lines 90 to 94 in b063518 [https://github.com/squeak-smalltalk/squeak-app/commit/b063518c61711ab7a0ca89ffa3d2494925fac823]
# Temporarily disable test status check for trunk builds. Remove this check as
# soon as all tests are running in trunk (hopefully soon).
if is_trunk; then
return 0
fi

Couldn't we remove this edge case and run check_test_status *after* the deployment stuff in prepare.sh?

Best,
Christoph

[http://www.hpi.de/]
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Fabio Niephaus <lists at fniephaus.com [mailto:lists at fniephaus.com]>
Gesendet: Freitag, 17. April 2020 16:25:40
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518)
 
On Fri, Apr 17, 2020 at 4:10 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de [mailto:Christoph.Thiede at student.hpi.uni-potsdam.de]> wrote:

Do we really want to get exit code 0 before every single test passes?

Sorry, but I don't understand your question. Maybe because not all tests were passing [1]?

[1] https://travis-ci.org/github/squeak-smalltalk/squeak-app/jobs/676189826#L789 [https://travis-ci.org/github/squeak-smalltalk/squeak-app/jobs/676189826#L789]
 
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von Travis CI <builds at travis-ci.org [mailto:builds at travis-ci.org]>
Gesendet: Freitag, 17. April 2020 15:41:44
An: squeak-dev at lists.squeakfoundation.org [mailto:squeak-dev at lists.squeakfoundation.org]
Betreff: [squeak-dev] [CRON] Fixed: squeak-smalltalk/squeak-app#1648 (squeak-trunk - b063518)
 
squeak-smalltalk
/
squeak-app
[https://travis-ci.org/github/squeak-smalltalk/squeak-app?utm_medium=notification&utm_source=email]
[branch icon]squeak-trunk [https://github.com/squeak-smalltalk/squeak-app/tree/squeak-trunk]
[build has passed]
Build #1648 was fixed [https://travis-ci.org/github/squeak-smalltalk/squeak-app/builds/676189824?utm_medium=notification&utm_source=email]
[arrow to build time]
[clock icon]20 mins and 26 secs
[Fabio Niephaus avatar]Fabio Niephaus
b063518 CHANGESET → [https://github.com/squeak-smalltalk/squeak-app/compare/76ccf898d33362c1a577fb2cccdcb3dd4ae9b253...b063518c61711ab7a0ca89ffa3d2494925fac823]
Send email notifications to squeak-dev [ci skip]
Want to know about upcoming build environment updates?
Would you like to stay up-to-date with the upcoming Travis CI build environment updates? We set up a mailing list for you!
SIGN UP HERE [http://eepurl.com/9OCsP]
[book icon]
Documentation [https://docs.travis-ci.com/] about Travis CI
Have any questions? We're here to help. [mailto:support at travis-ci.com]
Unsubscribe [https://travis-ci.org/account/preferences/unsubscribe?repository=8901856&utm_medium=notification&utm_source=email] from build emails from the squeak-smalltalk/squeak-app repository.
To unsubscribe from all build emails, please update your settings [https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email].
[black and white travis ci logo] [https://travis-ci.com]
Travis CI GmbH, Rigaer Str. 8, 10427 Berlin, Germany | GF/CEO: Randy Jacops | Contact: contact at travis-ci.com [mailto:contact at travis-ci.com] | Amtsgericht Charlottenburg, Berlin, HRB 140133 B | Umsatzsteuer-ID gemäß §27 a Umsatzsteuergesetz: DE282002648




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200427/8b743191/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image.png
Type: image/png
Size: 19029 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200427/8b743191/attachment-0001.png>


More information about the Squeak-dev mailing list