bugGNU Octave - Bugs: bug #58393, (buildbot) Tests are marked as...

 
 

bug #58393: (buildbot) Tests are marked as "pass" if Octave crashes

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Mon 18 May 2020 03:02:18 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 20 Jun 2020 04:48:53 PM UTC, comment #10: 

I hope it is ok that I'm closing this bug as fixed.

I still don't understand why the crashes during the test suite are marked as having run only one single test. But that's ok with me. That makes them even better distinguishable from jobs where just a few BISTs failed (without crashing).
It's a feature by coincidence. ;-)

Markus Mützel <mmuetzel>
Group administrator
Fri 12 Jun 2020 07:54:55 PM UTC, comment #9: 

The crashing build was a coincidence.

I think the bad counts are due to searching for "FAIL", "PASS", and "REGRESSION" to get those numbers and not handling the possibility that they are missing.


John W. Eaton <jwe>
Group administrator
Fri 12 Jun 2020 06:12:10 PM UTC, comment #8: 

Thanks.
That seems to be working now.
Shortly after you made the change, a builder crashed and it is marked as failed:
http://buildbot.octave.org:8010/#/builders/19/builds/1674/steps/7/logs/stdio
Other builders whose tests have previously been mark as failed (erroneously) are now marked as pass.

Was the crashing builder a coincidence? Or did you somehow provoke the crash to check it is working now?

I don't understand why the crashing buildbot is marked as having run only one single test. Why didn't it count the previous BISTs that ran before the crash?
But still, the current behavior is better than what we had before imho.

Markus Mützel <mmuetzel>
Group administrator
Fri 12 Jun 2020 03:33:04 PM UTC, comment #7: 

I made the change in the config file.  I reloaded the service.  Not sure whether that is enough.  If not, I'll try restarting it.

Now I'm wondering whether we should just fix "make check" to exit with a failure status if Octave crashes?  I'm surprised that doesn't already happen.

John W. Eaton <jwe>
Group administrator
Sat 06 Jun 2020 11:57:43 AM UTC, comment #6: 

I probably spoke too soon in my previous comment.
Scrolling through the log, I can see that the newer buildbot version seems to display output to stderr in red (or was this always the case and I just never noticed?).
It looks like there is actually a lot of output to stderr in a normal run of the test suite.

With the attached patch, I try to react only on lines that contain the word "fatal" (case-insensitive).
I'm not proficient at all in python. So there is probably a better way to do this.

(file #49219)

Markus Mützel <mmuetzel>
Group administrator
Sat 06 Jun 2020 11:10:13 AM UTC, comment #5: 

Some test are marked as failed now even if there are no failing tests. This is most probably due to this change.

Even if this is not exactly what we wanted, at least it looks like the function executes now.

It might be a coincidence. But the tests that are marked as failing are the ones with warnings. The ones without warnings are not marked as failing.
So maybe the warnings are emitted on stderr. In this case, we'd probably need to check if the errLine contains the word "warning" and not count it if it does.
I'll try and create a changeset for that.

Markus Mützel <mmuetzel>
Group administrator
Fri 05 Jun 2020 05:08:10 PM UTC, comment #4: 

I made the change in the master config file and restarted.

John W. Eaton <jwe>
Group administrator
Sat 30 May 2020 02:59:57 PM UTC, comment #3: 

I didn't check the repository or I would have noticed that you already caught that error.

I believe indentation matters in python. Does the attached patch make a difference?

(file #49187)

Markus Mützel <mmuetzel>
Group administrator
Tue 26 May 2020 05:36:29 PM UTC, comment #2: 

There is a typo in the code snippet in comment #0. It should read "errLineReceived", not "errLineRecived":
http://docs.buildbot.net/current/developer/cls-logobserver.html#buildbot.process.logobserver.LogLineObserver

Sorry for that.

Not sure this will make any difference however.

Markus Mützel <mmuetzel>
Group administrator
Tue 26 May 2020 04:25:22 PM UTC, comment #1: 

Since May 13 when I last restarted the buildbot master, we have been running with this change and it unfortunately doesn't seem to flag the segfaults as failures.

I'm also not an expert in buildbot and I don't know what change to make.

BTW, I noticed that my workers were at version 2.4 and 2.7 but the master was at 1.1.1.  I updated all to 2.7 this morning.  I think I was ultimately successful with that, but it required more time that it probably should have.

John W. Eaton <jwe>
Group administrator
Mon 18 May 2020 03:02:18 PM UTC, original submission:  

Like already reported on the mailing list a while ago:
https://octave.1599824.n4.nabble.com/buildbots-False-pass-results-for-segmentation-fault-in-test-td4695266.html

Quoting here:

> It looks like a segmentation fault in the last step of running the test suite on the build bots is still marked as "success" if there were no failed test up to that point. E.g.:
> http://buildbot.octave.org:8010/#/builders/12/builds/1525/steps/6/logs/stdio
>
> Maybe any output to stderr in that step should result in it to be marked as "failure"?


And a potential fix (but keep in mind that I don't know much about buildbot configuration):

diff -r e02079ee69d1 master.cfg
--- a/master.cfg Fri Feb 08 15:58:06 2019 +0000
+++ b/master.cfg Mon Jan 13 19:44:13 2020 +0100
@@ -324,6 +324,7 @@
     self.passed = 0
     self.failed = 0
     self.warnings = 0
+    self.errors = 0
     self.summary_found = False
     self.complete = False

@@ -352,6 +353,11 @@
           self.rc = FAILURE
           self.failed += num

+  def errLineRecived (self, line)
+    self.total += 1
+    self.errors += 1
+    self.rc = FAILURE
+

 class octave_test (Test):

@@ -365,7 +371,7 @@
       passed = self.observer.total - self.observer.failed

       self.setTestResults (total = self.observer.total,
-                           failed = self.observer.failed,
+                           failed = self.observer.failed + self.observer.errors,
                            passed = self.observer.passed,
                            warnings = self.observer.warnings)


Markus Mützel <mmuetzel>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #49219:  bug58393_fatal.patch added by mmuetzel (983B - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-20 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-06-06 mmuetzel Attached File- Added bug58393_fatal.patch, #49219
    2020-06-05 jwe StatusConfirmed Ready For Test
    2020-05-30 mmuetzel Attached File- Added bug58393_logline_observer_err.patch, #49187
    2020-05-18 rik5 Carbon-CopyRemoved 72865 -
    2020-05-18 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code