-
Notifications
You must be signed in to change notification settings - Fork 166
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
citgm-smoker-nobuild fails to start on Node 6 and 8 #1602
Comments
There is a bug with the groovy script in the job. I tried to fix it as part of #1577 but groovy script edits in Jenkins jobs require admin approval (being a member of citgm-admins is not sufficient). I can't access the CI at the moment since it's locked down for the security releases. |
It may be possible to work around the bug by specifying a more exact version (e.g. |
This comment has been minimized.
This comment has been minimized.
Well this works: def verString = parameters['NODE_VERSION'].toString()
println "using NODE_VERSION: " + verString
def sliced = verString.substring(1)
println "sliced: $sliced"
def spaced = spaced.replace(".", " ")
def split = sliced.split()
println "split : $split"
nodeMajorVersion = split[0].toInteger() |
And BTW |
BTW2 I restored fetching the groovy script from GitHub which I hacked in #1509 (comment) |
Instead of the slicing/replacing, we could always use a regexp to extract the major version. Something like: def nodeMajorVersion = (verString =~ /v?(\d+)/)[0][1] |
I think I don't like Groovy 😢 IMO what we should do is rename the job Parameters like I did in https://ci.nodejs.org/job/refack-node-test-node-addon-api/, and use numbers instead of strings (with no |
That sounds more or less what I did with the node-report job: #1421 |
@refack can I get you to hold off on the citgm job testing please? I need to get all these security jobs through and your testing is blocking that. I'd really like a pause on all non-security-release-related CI activity to be honest. There's a ton to do including fixing small bugs across all release lines and getting to citgm when things are stable. |
Abort them. The pipe line just schedules a ton of job. |
I feel bad cancelling people's stuff, it's always hard to know how critical something is or how much I'm going to ruin their day. Thanks for cancelling, it turns out that I have a small, but extremely hairy yak to shave and it's taking a lot of jenkins time to get it done. |
@refack we are testing it in AppVeyor, Node v6, v8, v10, v11 both x86 and x64. Why it fails on CITGM? What tests are failing? |
Had to fix up the matrix axis name ( Anyway I think the reported issue has now been fixed? |
Seems fixed indeed |
See the runs from https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/46/console
/cc @richardlau
The text was updated successfully, but these errors were encountered: