-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
Baseline rechecking is confusing and its failures are difficult to detect in sktm. Skt shouldn't really be doing it, since it wasn't skt which decided to check baseline in the first place.
Pull Request Test Coverage Report for Build 1005
💛 - Coveralls |
While I agree with the sentiment of the patch, I want to be sure this won't cause us report false failures. IIRC we are passing the latest stable baseline from sktm, so the baseline shouldn't fail here. Unless it's a host-specific failure which occurs with baseline as well (and these chosen hosts weren't tested when the baseline was marked as stable), which is what this chunk of the code is trying to rule out. What are the chances that we randomly get two hosts which fail and need to verify if it's the hosts or the patches that are broken (or even tests, which should be caught here too)? |
Let me just say this first: there are no records of baseline failures in our database at all. I.e. they were never detected. I don't think I've ever seen a Beaker job for a baseline check either. Now, the code being removed is making an assumption that the baseline build is going to be present at the same location the build it is testing is published at, which is not skt's business to know, at all. The proper (maintainable and usable) way of implementing this would be to explicitly supply skt with the baseline build (in Beaker runner parameters), and to let skt report ERROR (not PASS) if it fails to find a host where baseline works. Something along that line, anyway, and not what's being done right now. |
I don't think the DB entries are relevant. What the code does is "everything failed, let me see if it's bug with the series or not". If this chunk of code never ran, then it answers my question about the probability and I'm fine with the removal :) Agreed with the rest of your comment. That said, should that change be implemented as well? |
We would be better off "running light" for the report refactoring and adding baseline checking afterwards, lest it slows us down. |
Agreed. Can you submit an issue that would describe the problem and possible solution so we can track it and not forget about it, and I'll ack this patch? |
@veruu, would cki-project/meta#7 do? |
Yup! Thank you! |
Baseline rechecking is confusing and its failures are difficult to
detect in sktm. Skt shouldn't really be doing it, since it wasn't skt
which decided to check baseline in the first place.