Skip to content

ncu-ci: parse console text of the first failed phase #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2019

Conversation

joyeecheung
Copy link
Member

Previously we try to detect the type of the failed phase
and return a generic build failure when the job failed in a
non-test phase. This is not really necessary - the console
text parser is capable of detecting the actual build failure
from even the console text of the build phase. Also hard-coding
the test phase name makes the detection fragile when there
are configuration changes in the Jenkins. So we should just
skip the detection.

Fixes: #344

Previously we try to detect the type of the failed phase
and return a generic build failure when the job failed in a
non-test phase. This is not really necessary - the console
text parser is capable of detecting the actual build failure
from even the console text of the build phase. Also hard-coding
the test phase name makes the detection fragile when there
are configuration changes in the Jenkins. So we should just
skip the detection.
@joyeecheung
Copy link
Member Author

cc @nodejs/automation

@codecov
Copy link

codecov bot commented Jun 12, 2019

Codecov Report

Merging #351 into master will increase coverage by 0.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #351      +/-   ##
==========================================
+ Coverage    75.8%   75.95%   +0.14%     
==========================================
  Files          21       21              
  Lines        1368     1364       -4     
==========================================
- Hits         1037     1036       -1     
+ Misses        331      328       -3
Impacted Files Coverage Δ
lib/ci/ci_result_parser.js 48.81% <ø> (+0.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 286a61e...11527af. Read the comment docs.

Copy link

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rather fix this at the json side of things?

@joyeecheung
Copy link
Member Author

@evenstensberg What do you mean by json side of things?

@evenstensberg
Copy link

I’m just trying to understand the source of truth. New to the automation team so trying to get insight

@joyeecheung
Copy link
Member Author

@evenstensberg The source of truth is Jenkins. This tool adapts to the configurations there.

@evenstensberg
Copy link

Okay so can't change output instead of removing the test?

CC @mhdawson for reviews

@joyeecheung
Copy link
Member Author

joyeecheung commented Jun 12, 2019

@evenstensberg There isn't any test actually removed here? It simply removes an false early return and always run the logic below no matter how the phases are named.

@evenstensberg
Copy link

Okay gotcha, thanks for elaborating!

@joyeecheung joyeecheung merged commit 8d64553 into nodejs:master Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ncu-ci stopped working on Windows jobs
5 participants