-
Notifications
You must be signed in to change notification settings - Fork 35
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
Unable to release due to test failures #754
Comments
@loicmathieu same issue again I think? There is no logging of the actual docker container, which fails to start, so this is a bit of an educated guess ( I need to fix this in the future somehow..) , but if I scan through the workflow files, the npm install step is missing there, which would mean the docker container won't boot up due to the lack of node_modules. So I think we can just add that npm install step, just need to figure out where to put it (haven't dived into the release process yet and it incudes quite a bit of other files, i.e. if you have a pointer, that would help getting it fixed quickly) |
Would adding a script that would get invoked here https://github.com/quarkiverse/.github/blob/c4f83117e90e92b931ea21c7fcec4c5b991b0169/.github/workflows/prepare-release.yml#L102 work? And then in the script first install NPM (as that is not present in the base image) and after that do an npm install? So far that seems to only hook we have? (Btw. for the future I'm thinking about using something like the fabri8 docker maven plugin to run that NPM install from maven, avoiding all the workflow changes which seems kinda brittle, also from a local dev perspective, as you need to know you need to run npm install in advance or else the build will fail. But that;s probably a bit of a stretch to get running so short before release) |
So this is quite annoying as this prepare-release step is a step coming from the parent Quarkiverse release mechanism. We may have the same issue for perform-release :( But I think the best would be to start node from Maven, for ex with a container using Fabric8 https://maven.fabric8.io/ |
Example of the Fabric8 Maven plugin in Quarkus: https://github.com/quarkusio/quarkus/blob/main/extensions/reactive-pg-client/deployment/pom.xml#L134 |
Yeah exactly, main reason to go for that fabri8 approach, way easier on all the maven steps and way easier for anyone working on this. I'll have a look at implementing this, tnx for the reference, up to you if you want to delay the release a bit or just disable the test for now. Hope to get this done somewhere this week, but it's a bit time permitting.. |
I didn't remember when Quarkus 3.18 will be released but it may be very soon so better to release without the test or it will not be part of the next platform release |
Just created an initial version of the fix which might just work. Build is running right now. Once that succeeds I can remove the node stuff from the workflow files. Once done if you could start those ecosystem builds, we might get this running today |
@loicmathieu build succeeded, just pushed the change for the ecosystem CI, can you give that one a kick? |
Failed again I see? Maybe just go ahead with out that test, would need more investigation to see why it's failing. @loicmathieu |
Yes, I disabled both test until we find a solution. |
https://github.com/quarkiverse/quarkus-google-cloud-services/actions/runs/12992225677/job/36274041597
@jfbenckhuijsen if you can have a look, otherwise I'll disable this test for now...
The text was updated successfully, but these errors were encountered: