Skip to content

[AI-8th] RRefactor auto-configuration registration migration checks to guard AutoConfiguration.imports#1420

Open
pmupkin wants to merge 1 commit intosofastack:masterfrom
pmupkin:migrate-factories
Open

[AI-8th] RRefactor auto-configuration registration migration checks to guard AutoConfiguration.imports#1420
pmupkin wants to merge 1 commit intosofastack:masterfrom
pmupkin:migrate-factories

Conversation

@pmupkin
Copy link
Copy Markdown

@pmupkin pmupkin commented Apr 23, 2026

Summary

This PR(Related issue #1401) reinforces the migration from legacy spring.factories auto-configuration registration to META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports.

The current codebase has already moved the actual auto-configuration registrations into .imports files for the relevant modules. This change adds regression coverage to make sure we do not accidentally reintroduce EnableAutoConfiguration entries in spring.factories in the future.

Changes

  • Add AutoConfigurationMigrationTests in sofa-boot
  • Verify that every main module containing *AutoConfiguration classes provides an AutoConfiguration.imports file
  • Verify that each AutoConfiguration.imports file matches the actual set of auto-configuration classes in that module
  • Verify that main-module META-INF/spring.factories files no longer register org.springframework.boot.autoconfigure.EnableAutoConfiguration

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.94%. Comparing base (459b02f) to head (ca9ced7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1420   +/-   ##
=========================================
  Coverage     82.94%   82.94%           
  Complexity     2975     2975           
=========================================
  Files           340      340           
  Lines          9833     9833           
  Branches       1178     1178           
=========================================
  Hits           8156     8156           
  Misses         1163     1163           
  Partials        514      514           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a regression test in the sofa-boot module to enforce the project’s migration away from legacy META-INF/spring.factories EnableAutoConfiguration registration and toward META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports.

Changes:

  • Add AutoConfigurationMigrationTests to validate every module with *AutoConfiguration classes provides an AutoConfiguration.imports file.
  • Validate each AutoConfiguration.imports file matches the discovered *AutoConfiguration classes.
  • Validate main-module spring.factories files do not register org.springframework.boot.autoconfigure.EnableAutoConfiguration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +47 to +50
Set<Path> autoConfigurationModules = findMainSourceDirectories(PROJECT_ROOT).stream()
.map(this::toModuleDirectory)
.filter(this::containsAutoConfigurationSource)
.collect(Collectors.toCollection(TreeSet::new));
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modulesWithAutoConfigurationsShouldProvideImportsFile() maps src/main/java directories to the module dir and then calls containsAutoConfigurationSource(moduleDir), which walks the whole module (including src/test/resources). This can produce false positives (e.g., a test-only *AutoConfiguration.java) and does extra IO. Consider scanning only the src/main/java directory (e.g., filter before toModuleDirectory, or have containsAutoConfigurationSource take/resolve the main source directory).

Suggested change
Set<Path> autoConfigurationModules = findMainSourceDirectories(PROJECT_ROOT).stream()
.map(this::toModuleDirectory)
.filter(this::containsAutoConfigurationSource)
.collect(Collectors.toCollection(TreeSet::new));
Set<Path> autoConfigurationModules = new TreeSet<>();
for (Path mainSourceDirectory : findMainSourceDirectories(PROJECT_ROOT)) {
if (!findAutoConfigurationClasses(mainSourceDirectory).isEmpty()) {
autoConfigurationModules.add(toModuleDirectory(mainSourceDirectory));
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +140 to +156
private List<Path> findFiles(Path root, String suffix) throws IOException {
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isRegularFile)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.toString().endsWith(suffix))
.sorted()
.collect(Collectors.toList());
}
}

private List<Path> findMainSourceDirectories(Path root) throws IOException {
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isDirectory)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.toString().endsWith("src/main/java"))
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findFiles(...) uses path.toString().endsWith(suffix) where suffix contains /. This will not match on Windows (which uses \), causing the test to miss files and fail. Use Path-aware matching (e.g., path.endsWith(Path.of(suffix)) or compare path segments) instead of string suffix matching.

Suggested change
private List<Path> findFiles(Path root, String suffix) throws IOException {
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isRegularFile)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.toString().endsWith(suffix))
.sorted()
.collect(Collectors.toList());
}
}
private List<Path> findMainSourceDirectories(Path root) throws IOException {
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isDirectory)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.toString().endsWith("src/main/java"))
private Path toRelativePath(String suffix) {
return Path.of("", suffix.split("/"));
}
private List<Path> findFiles(Path root, String suffix) throws IOException {
Path suffixPath = toRelativePath(suffix);
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isRegularFile)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.endsWith(suffixPath))
.sorted()
.collect(Collectors.toList());
}
}
private List<Path> findMainSourceDirectories(Path root) throws IOException {
Path mainSourceDirectory = toRelativePath("src/main/java");
try (Stream<Path> files = Files.walk(root)) {
return files.filter(Files::isDirectory)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.endsWith(mainSourceDirectory))

Copilot uses AI. Check for mistakes.
return files.filter(Files::isDirectory)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.filter(path -> path.toString().endsWith("src/main/java"))
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findMainSourceDirectories(...) relies on path.toString().endsWith("src/main/java"), which is also path-separator dependent and will fail on Windows. Prefer path.endsWith(Path.of("src","main","java")) (or similar segment-based checks) so the test is platform-independent.

Suggested change
.filter(path -> path.toString().endsWith("src/main/java"))
.filter(path -> path.endsWith(Path.of("src", "main", "java")))

Copilot uses AI. Check for mistakes.
@CrazyHZM
Copy link
Copy Markdown
Member

@pmupkin Please fix the comment from AI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants