Skip to content

Fix build with Windows #958

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

Closed
fabapp2 opened this issue Oct 9, 2023 · 6 comments · Fixed by #969 or #965
Closed

Fix build with Windows #958

fabapp2 opened this issue Oct 9, 2023 · 6 comments · Fixed by #969 or #965

Comments

@fabapp2
Copy link
Contributor

fabapp2 commented Oct 9, 2023

What needs to be done

Fix the build of main branch under Windows.

Why it needs to be done

After getting my hands on a Windows machine it turned out the build fails.

@ammachado
Copy link
Contributor

One thing I noticed in my tests here is that there are a lot of ant path patterns starting with "/" (for instance, on MoveFilesActionTest class).

I think that these patterns should be using relative patterns (such as **/SomeFile.foo, not /**/SomeFile.foo).

This was linked to pull requests Oct 11, 2023
@fabapp2
Copy link
Contributor Author

fabapp2 commented Oct 16, 2023

I can't recall. I think I had problems with relative paths.
Currently, I have no access to a Win machine but when #965 is merged these patterns should be working.

@bottemav
Copy link
Contributor

I tested the branch of @ammachado on Windows but there are still failing tests, for example:
[ERROR] Module_searchMainJava_Test$MultiMavenModuleProject.withClassesInMainAndTest_providesClassesFromSrcMainJava:219->lambda$withClassesInMainAndTest_providesClassesFromSrcMainJava$2:222 expected: "application/src/main/java/SomeClass.java" but was: "application\src\main\java\SomeClass.java" [ERROR] Module_searchMainJava_Test$MultiMavenModuleProject.withClassesInMain_providesClassesFromSrcMainJava:236->lambda$withClassesInMain_providesClassesFromSrcMainJava$3:239 expected: "application/src/main/java/SomeClass.java" but was: "application\src\main\java\SomeClass.java"

The failing tests compare the toString method of getSourcePath, for example
assertThat(projectResourceSet.list().get(0).getSourcePath().toString()).isEqualTo("application/src/main/java/SomeClass.java");

Should I add a method String getUnifiedSourcePath to RewriteSourceFileHolder?
The fixes @fabapp2 did on the methods getSourcePath() and getAbsolutePath doesn't seem to work as the return type is Path, not String. And Path uses the default file system separator.

An alternative is to fix it in the tests themselves, so I don't need to touch RewriteSourceFileHolder:
assertThat(LinuxWindowsPathUnifier.unifyPath(projectResourceSet.list().get(0).getSourcePath())).isEqualTo("application/src/main/java/SomeClass.java");

@ammachado
Copy link
Contributor

ammachado commented Oct 21, 2023

@bottemav I applied the changes you mentioned, can you test again? I'm away from my Windows machine until November.

@bottemav
Copy link
Contributor

@ammachado Thanks for the quick fix. The build now fails due to this test

[ERROR] org.springframework.sbm.project.parser.ForgivingParsingOfTestResourcesTest.test_renameMe -- Time elapsed: 0.410 s <<< ERROR!
java.lang.RuntimeException: 
src\test\resources\error.yaml mapping values are not allowed here
 in 'reader', line 4, column 21:
      risk-score-classes: !include risk-score-class_ok.y ... 
                        ^
	at org.springframework.sbm.openrewrite.RewriteExecutionContextErrorHandler.accept(RewriteExecutionContextErrorHandler.java:47)
	at org.springframework.sbm.openrewrite.RewriteExecutionContextErrorHandler.accept(RewriteExecutionContextErrorHandler.java:27)
	at org.openrewrite.yaml.YamlParser.lambda$parseInputs$0(YamlParser.java:84)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at org.openrewrite.yaml.YamlParser.parseInputs(YamlParser.java:98)
	at org.springframework.sbm.project.parser.ResourceParser.parseSingleResource(ResourceParser.java:150)
	at org.springframework.sbm.project.parser.ResourceParser.lambda$getSourceFileStream$8(ResourceParser.java:143)
        ...
Caused by: java.lang.IllegalStateException: src\test\resources\error.yaml mapping values are not allowed here
 in 'reader', line 4, column 21:
      risk-score-classes: !include risk-score-class_ok.y ... 
                        ^
	... 106 more
Caused by: mapping values are not allowed here
 in 'reader', line 4, column 21:
      risk-score-classes: !include risk-score-class_ok.y ... 
                        ^

I then fixed ResourceParser and ForgivingParsingOfTestResourcesTest, but after that 5 other tests fail:

  • AddSpringBootContextTestClassTest$GivenSingleModuleProject.testApplyShouldAddNewSource:94
  • AddSpringBootMainClassActionTest.testApplyShouldAddNewSource:56
  • SpringBeanMethodDeclarationFinderTest$WithMatchingBean.shouldReturnTheMatchingBeanDeclaration:81
  • SpringBeanMethodDeclarationFinderTest$WithMultipleMatchingBeans.shouldReturnTheMatchingBeanDeclarations:151
  • InitializeSpringBootMigrationRecipeIntegrationTest.initializeBlankProjectAsSpringBootApplication:29

What is the best approach to proceed?

fabapp2 added a commit that referenced this issue Oct 30, 2023
Enhancements to #958

---------

Co-authored-by: Adriano Machado <[email protected]>
Co-authored-by: Fabian Krüger <[email protected]>
Co-authored-by: Adriano Machado <[email protected]>
@fabapp2 fabapp2 mentioned this issue Nov 3, 2023
fabapp2 added a commit that referenced this issue Nov 20, 2023
- Build runs under Windows
- All tests green
- Shell application works
- Spring Boot 3 Upgrade Report app works 

Changes
- Change line endings to sue `\n` and adjust string comparisons in tests
- Fix GH action
- Add new method `get...Path()` to `ProjectResource`

closes #958

---------

Co-authored-by: Adriano Machado <[email protected]>
Co-authored-by: Adriano Machado <[email protected]>
Co-authored-by: Adriano Machado <[email protected]>
@ammachado
Copy link
Contributor

@fabapp2 the Windows build is still broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants