eclipse / microprofile-health

microprofile-health
Apache License 2.0
106 stars 61 forks source link

[issue-325] - Add tests to verify that HealthCHeckResponse has a name when built as recommended on the producer side #326

Open fabiobrz opened 6 months ago

fabiobrz commented 6 months ago

SSIA.

Fixes #325

Draft - still running TCKs

fabiobrz commented 6 months ago

Thanks @fabiobrz for your PR. But I need to request some changes. I don't think generating the name should be our solution to null names. I don't see the point of having random name since it won't (probably) give value to user when the health checks is first invoked. So then when then need to restart the app to add the name, I think it would be better to throw and exception (possibly on startup when we detect null name) to prevent app from running.

Yeah, you might be right in here, thanks, fixing.

Also in every case, this will require adjustment in the specification text. I'm willing to hop on a quick call if you want to discuss this case in more detail. (/cc @pgunapal I would like to hear you opinion too).

Correct, the spec text should be adjusted. I am glad to learn about this, so feel free to get in touch, and thanks for your review!

pgunapal commented 6 months ago

I agree with @xstefank, It wouldn't make sense to randomly generate a name for a Health check with no-name. We should throw an exception if we detect a health check without a name.

+1 on updating the specification, with this detail.