Improve calculating preferred machine #2249
Conversation
Martchus
commented
Aug 8, 2019
Martchus
commented
- Make result deterministic in case multiple machines occur equally often (see https://progress.opensuse.org/issues/16354)
- Improve comments
* Make result deterministic in case multiple machines occur equally often (see https://progress.opensuse.org/issues/16354) * Improve comments
Codecov Report
@@ Coverage Diff @@
## master #2249 +/- ##
==========================================
- Coverage 86.1% 86.03% -0.08%
==========================================
Files 166 166
Lines 10825 10826 +1
==========================================
- Hits 9321 9314 -7
- Misses 1504 1512 +8
Continue to review full report at Codecov.
|
2 similar comments
Codecov Report
@@ Coverage Diff @@
## master #2249 +/- ##
==========================================
- Coverage 86.1% 86.03% -0.08%
==========================================
Files 166 166
Lines 10825 10826 +1
==========================================
- Hits 9321 9314 -7
- Misses 1504 1512 +8
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #2249 +/- ##
==========================================
- Coverage 86.1% 86.03% -0.08%
==========================================
Files 166 166
Lines 10825 10826 +1
==========================================
- Hits 9321 9314 -7
- Misses 1504 1512 +8
Continue to review full report at Codecov.
|
| @@ -393,24 +393,25 @@ sub job_next_previous_ajax { | |||
|
|
|||
| sub _calculate_preferred_machines { | |||
There was a problem hiding this comment.
Can we add a unit test for this somewhere? This looks to be completed untested? At least in the unit tests I can't find it...
There was a problem hiding this comment.
It seems to be covered by tests somehow at least implicitly because codecov reports for the diff to be completely covered :)
|
This is covered and only checks are missing. I don't want to add checks for this behavior which seems arbitrarily too me. With this PR it at least always takes the same arbitrary choice all the time. It still seems weird to allow the most popular machine to be omitted and then merge the rows. So I honestly don't understand what the purpose of this "preferred" machine thing is. To me it would actually look best and most space-efficient if all architectures for one test would be in a single row and the machine wouldn't be mentioned at all on this level or maybe only in a tooltip. |
|
Ok, in this overview it actually make sense: https://openqa.suse.de/tests/overview?result=failed&arch=&modules=&distri=sle&version=12-SP5&build=0216&groupid=240 (link from the duplicated ticked https://progress.opensuse.org/issues/54008) Here the machine differs between jobs of the same architecture. So the two |
Since people ran into this and reported it, I think that makes it worth testing that it works as expected. However I second re-evaluating if this is the behavior we want. It reminds me of the original generated YAML which was too smart and ended up being confusing even when I made it deterministic. |
|
I personally would prefer to merge this and think of checks later. |
|
IMHO making the current behaviour deterministic is highest priority. Let's discuss general alternatives to "preferred machine" not in this PR, ok? |
kalikiana
dismissed
their stale review
I don't see the urgency here... I'll abstain
