Skip to content
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

HelmServiceDependencyUpdateIT is failing on windows #3121

Closed
Sintivrousai opened this issue May 31, 2024 · 6 comments · Fixed by #3140
Closed

HelmServiceDependencyUpdateIT is failing on windows #3121

Sintivrousai opened this issue May 31, 2024 · 6 comments · Fixed by #3140
Assignees
Milestone

Comments

@Sintivrousai
Copy link
Contributor

Sintivrousai commented May 31, 2024

Part of #1338

HelmServiceDependencyUpdateIT is failing on windows.

[ERROR] Failures:
[ERROR] HelmServiceDependencyUpdateIT$ValidChart.whenConfigurationOptionsProvided_thenDependencyDownloaded:97->verifyHelmDependencyDownloaded:113
Argument(s) are different! Wanted:

Can I work on that?

@rohanKanojia rohanKanojia changed the title HelmServiceDependencyUpdateIT is failing HelmServiceDependencyUpdateIT is failing on windows May 31, 2024
@rohanKanojia
Copy link
Member

Hello, Sorry I didn't test this on windows while adding this test in #3098

Here are some code pointers to fix the test failures:

  • whenInvalidChartDirProvided_thenThrowException : Test is failing due to a platform specific error message, maybe we can run this test only for Linux by adding @EnabledOnOs(OS.LINUX) annotation. You can also add same test that verifies message on Windows but that would only run on windows @EnabledOnOs(OS.WINDOWS)
  • Other tests are failing due to helm CLI returning output with Line Feeds \n instead of CLRF \r\n on Windows. Here is screenshot
    Screenshot 2024-06-04 213002

Here is the code that takes output of helm command and splits it based on System.lineSeparator() :

Arrays.stream(dependencyUpdateCommand.call()
.split(System.lineSeparator()))
.forEach(l -> logger.info("[[W]]%s", l));
}

Test should pass when we update above mentioned code like this:

      String dependencyUpdateCommandOutput = dependencyUpdateCommand.call();
      String lineDelimiter = System.lineSeparator();
      if (!dependencyUpdateCommandOutput.contains(System.lineSeparator())) {
        lineDelimiter = "\n";
      }
      Arrays.stream(dependencyUpdateCommandOutput
          .split(lineDelimiter))
          .forEach(l -> logger.info("[[W]]%s", l));

@manusa
Copy link
Member

manusa commented Jun 5, 2024

Have we tried this:

   Arrays.stream(dependencyUpdateCommand.call() 
       .split("\r?\n") 
       .forEach(l -> logger.info("[[W]]%s", l)); 
 } 

@rohanKanojia
Copy link
Member

Yeah, this is better.

@Sintivrousai
Copy link
Contributor Author

Yeah, that part is okey but I'm encounering another issue.

[ERROR] Failures:
[ERROR] HelmServiceDependencyUpdateIT.whenInvalidChartDirProvided_thenThrowException:125
Expecting throwable message:
"could not find C:\Users\sindi\AppData\Local\Temp\junit11440685796248551667\output\kubernetes: CreateFile C:\Users\sindi\AppData\Local\Temp\junit11440685796248551667\output\kubernetes: The system cannot find the path specified."
to contain:
"no such file or directory"
but did not.

The part of the test that causes the problem is this:

@test
@DisplayName("invalid chart provided, then throw exception")
void whenInvalidChartDirProvided_thenThrowException() {
// When + Then
assertThatIllegalStateException()
.isThrownBy(() -> helmService.dependencyUpdate(helmConfig))
.withMessageContaining("no such file or directory");
}

Is this test required because I can't see such part in the HelmService.dependencyUpdate()?

@rohanKanojia
Copy link
Member

@Sintivrousai see my comment #3121 (comment) . You can configure test to be run only in linux by adding @EnabledOnOs(OS.LINUX) annotation

  • whenInvalidChartDirProvided_thenThrowException : Test is failing due to a platform-specific error message, maybe we can run this test only for Linux by adding @EnabledOnOs(OS.LINUX) annotation. You can also add same test that verifies message on Windows but that would only run on windows @EnabledOnOs(OS.WINDOWS)

@Sintivrousai
Copy link
Contributor Author

oh i'm sorry i didn't see it

Sintivrousai added a commit to Sintivrousai/jkube that referenced this issue Jun 5, 2024
@manusa manusa added this to the 1.17.0 milestone Jun 6, 2024 — with automated-tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants