Return the last coverage in trace stream
What does this MR do?
This MR fixes the coverage issue raised at https://gitlab.com/gitlab-org/gitlab-ce/issues/31556. Currently, the first coverage appears in the trace will be considered as the coverage. Since general total scores will likely appear at the last part of logs, coverage extraction should be targeting the last matched part.
Are there points in the code the reviewer needs to double check?
Need to be checked by @godfat
Why was this MR needed?
Because #31556 (closed) labeled as regression
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #31556 (closed)
Merge request reports
Activity
@godfat Feel free to ignore, if you've already working on it
Reproduced at https://gitlab.com/dosuken123/gitlab-ce/builds/15703125 with https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11128/diffs?diff_id=4242643
Failures: 1) Gitlab::Ci::Trace::Stream#extract_coverage multiple results in content & regex returns the last matched coverage Failure/Error: is_expected.to eq("98.29") expected: "98.29" got: "98.39" (compared using ==) # ./spec/lib/gitlab/ci/trace/stream_spec.rb:253:in `block (4 levels) in <top (required)>' Finished in 31 minutes 27 seconds (files took 32.94 seconds to load) 1358 examples, 1 failure Failed examples: rspec ./spec/lib/gitlab/ci/trace/stream_spec.rb:252 # Gitlab::Ci::Trace::Stream#extract_coverage multiple results in content & regex returns the last matched coverage
Performance improvement
reverse_line
[22] pry(main)> puts Benchmark.measure { stream.extract_coverage('bdajlshaksdhal') } 0.010000 0.010000 0.020000 ( 0.017088) => nil
stream.each_line
[2] pry(main)> puts Benchmark.measure { stream.extract_coverage('bdajlshaksdhal') } 0.170000 0.070000 0.240000 ( 0.244303) => nil
Filesize: 3.4M
I'm thinking to add more tests as https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10736/diffs and close the MR...
added 303 commits
-
d8c4a24c...6ad3814e - 300 commits from branch
gitlab-org:master
- 9ae36e9f - Reproduced
- 6abf2750 - Add reverse_line
- 7b421ca5 - Fix while true
Toggle commit list-
d8c4a24c...6ad3814e - 300 commits from branch
added 303 commits
-
d8c4a24c...6ad3814e - 300 commits from branch
gitlab-org:master
- 9ae36e9f - Reproduced
- 6abf2750 - Add reverse_line
- 7b421ca5 - Fix while true
Toggle commit list-
d8c4a24c...6ad3814e - 300 commits from branch
@godfat Reproduced
and fixed pipeline passedHowever, I'd like to ask you to advice for more improvements. I've implemented a new function
reverse_line
, though, the actual process is fetching chunks (Not line!) of codes byBUFFER_SIZE
and extracting coverage bymatches.flatten.last
. I think this would be faster than iterating each line, though, I'm unconfident whether this approach is the best.- Resolved by Shinya Maeda
changed milestone to %9.2
added 71 commits
-
7b421ca5...f59a44db - 67 commits from branch
gitlab-org:master
- 7551c572 - Reproduced
- 912b44c9 - Add reverse_line
- 90e49fad - Fix while true
- 33e597a7 - Fix reverse_line from chunk based
Toggle commit list-
7b421ca5...f59a44db - 67 commits from branch
marked the checklist item Changelog entry added, if necessary as completed
marked the checklist item Conform by the merge request performance guides as completed