fix(template): correct return of String-encoded Optional #482
Conversation
|
/build_test |
|
Workflow started at 5/29/2024, 3:20:05 PM. View Actions Run. |
|
No OpenAPI schema changes detected. |
|
No GraphQL schema changes detected. |
|
CI build and push: At least one test failed ❌ (JDK17) |
|
I'm not sure what's up with the CI failure. It got to the point of running the test, but the test failed because the expected event template wasn't found. I'm not sure how that can be the case. This test passes locally, and I can't remember the exact reason I initially had this test disabled in CI only - I suppose it was also failing in CI, but I don't quite understand what that is the case. Maybe it has to do with the fact that the CI action passes the Maven property to not build a container image, but I would expect that to either cause the tests not to run or to run with an outdated image, and even a very outdated image should still contain that template file. https://github.com/cryostatio/cryostat3/blame/main/src/test/java/itest/CryostatTemplateIT.java#L35 |
|
/build_test |
|
Workflow started at 5/29/2024, 4:09:17 PM. View Actions Run. |
|
No GraphQL schema changes detected. |
|
No OpenAPI schema changes detected. |
|
CI build and push: At least one test failed ❌ (JDK17) |
|
It seems like if the image doesn't get built then the tests run with a local process version of Cryostat. The So once this is merged and the CI action change takes effect this should pass in CI as well. |
Welcome to Cryostat3! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/mainFixes: #481
Description of the change:
There was a bit of jumbled handling of an Optional in the implementation from #449, which resulted in an Optional that wrapped around another
Optional#toString()call erroneously. The original Optional itself is the expected return, not its String form.This got missed because the test doesn't run in CI. I think this is because of the Maven property to not build container images during CI. This probably made sense previously when there were no real tests to run and so building the container image as a throwaway was just a waste of CI time, but now there are integration tests to be run which do need the container image to have been built first.
How to manually test:
./mvnw clean verify ; podman image prune -f. All tests should pass./smoketest.bash -O, once the test comes up open the UI and start a recording on Cryostat itself usinglocalhost:0using the Cryostat template. Go to Security and define a stored credential that matchescryostat3:9091withcryostat:password, create another stored credential withfalseandtest:test, go back to Recordings and selectcryostat3:9091. Then download the recording you started and open it with JMC. Go to the Event Browser and verify that the events were captured.