Fix Unit Tests failing for windows in `jkube-kit/helm` module about jkube HOT 6 OPEN

@rohanKanojia, Kindly assign this issue to me.

from jkube.

@l3002 : Although I think Marc's opinion would also be same as yours. But it's okay to wait for a few hours till he comes back.

from jkube.

Please create a specific test for windows annotated with @EnabledOn... and disable the original test in windows with @DisabledOn...

from jkube.

Hi @rohanKanojia / @manusa : While working on this issue, I realized that in addition to the above mentioned problem with file separator, one more test was failing in HelmServiceUploadIT.

Test Failure at : HelmServiceUploadIT$OCI.withSuccessfulUpload_shouldUploadBlobsAndUpdateManifest()

The test was returning a BadUploadException : 404 while uploading the chart to the mock server. Upon investigation I found that it was occurring due to the change in message digest for Chart.yaml.

I suspect when we invoke Serialization.asJson(chart) after creating the Chart Object, it actually uses the platform dependent line separator (CRLF) for creating json with the data in Chart.yaml due to which when we create the byteArray it actually encodes the data accordingly and therefore the change in digest.

There can be two fixes to this issue. Either we can configure this test HelmServiceUploadIT$OCI.withSuccessfulUpload_shouldUploadBlobsAndUpdateManifest() by adding new mockServer.expect() statement and adding a new if statement before assertThat() statement to check if the OS is windows or Linux/Mac as due to the change in content of chart (LF -> CRLF) the digest and the size is altered. See below:

Or We can write a separate test for windows, where we will first configure mock server to expect the request with the message digest created for the windows chart and then add the assertThat() statement to test it. In this case, we will have to add @DisabledOnOs(OS.WINDOWS) and @DisabledOnOs(OS.LINUX)

I believe that the latter would be the best approach as when I tried to configure the test HelmServiceUploadIT$OCI.withSuccessfulUpload_shouldUploadBlobsAndUpdateManifest() to covering windows test, it seems messy and unreadable. Also, I'm not sure how to write a assertThat() statement for a specific platform. Just wanted to know what your thoughts are on the same.

from jkube.

In this case, we will have to add @DisabledOnOs(OS.WINDOWS) and @DisabledOnOs(OS.LINUX)

@l3002 : I agree with you. Adding conditionals for platform specific checks might make test less readble. It's better to write it as a separate test.

from jkube.

@rohanKanojia : Got it. So, Shall I start working on the PR or wait for @manusa's thought on this matter?

from jkube.