fix unit tests #540
fix unit tests #540
Conversation
sofastack-cla
bot
added
bug
WalkthroughThe pull request encompasses several updates primarily focused on dependency management within various Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tracer-sofa-boot-starter/src/main/java/com/alipay/sofa/tracer/boot/listener/SofaTracerConfigurationListener.java (1)
Line range hint
34-40: Enhance class documentation for dependency requirements.Given the critical role of this listener in early configuration and its dependency on
sofa-boot, consider enhancing the class documentation to:
- Document the required
sofa-bootdependency- Explain the reasoning behind the execution order (HIGHEST_PRECEDENCE + 30)
Add the following to the class documentation:
/** * Parse SOFATracer Configuration in early stage. * + * Requires com.alipay.sofa:sofa-boot dependency for environment utilities. + * Executes with high precedence to ensure tracer configuration is available + * before other components initialize. * * @author qilong.zql * @since 2.2.2 */pom.xml (1)
Line range hint
1-516: Missing core dependency changeAccording to the PR objectives, this change should add
com.alipay.sofa:sofa-bootdependency to fix the unit tests, but this change is missing from the POM file.Add the following dependency to the
dependencyManagementsection:<dependencyManagement> <dependencies> + <dependency> + <groupId>com.alipay.sofa</groupId> + <artifactId>sofa-boot</artifactId> + <version>${sofa.boot.version}</version> + </dependency>Note: Please also define the
sofa.boot.versionproperty in thepropertiessection with the appropriate version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
- pom.xml (1 hunks)
- sofa-tracer-plugins/sofa-tracer-datasource-plugin/pom.xml (0 hunks)
- sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/test/java/com/sofa/tracer/plugins/datasource/base/BaseTest.java (1 hunks)
- sofa-tracer-plugins/sofa-tracer-kafkamq-plugin/pom.xml (0 hunks)
- sofa-tracer-plugins/sofa-tracer-rabbitmq-plugin/pom.xml (0 hunks)
- tracer-core/src/test/java/com/alipay/common/tracer/core/benchmark/CountBenchmark.java (0 hunks)
- tracer-sofa-boot-starter/pom.xml (2 hunks)
- tracer-sofa-boot-starter/src/main/java/com/alipay/sofa/tracer/boot/listener/SofaTracerConfigurationListener.java (2 hunks)
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/AbstractTestBase.java (1 hunks)
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/AbstractTestCloudBase.java (2 hunks)
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/ConfigurationHolderListener.java (2 hunks)
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/SpringBootWebApplication.java (2 hunks)
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/config/ConfigurationTest.java (0 hunks)
- tracer-test/core-test/pom.xml (0 hunks)
💤 Files with no reviewable changes (6)
- sofa-tracer-plugins/sofa-tracer-datasource-plugin/pom.xml
- sofa-tracer-plugins/sofa-tracer-kafkamq-plugin/pom.xml
- sofa-tracer-plugins/sofa-tracer-rabbitmq-plugin/pom.xml
- tracer-core/src/test/java/com/alipay/common/tracer/core/benchmark/CountBenchmark.java
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/config/ConfigurationTest.java
- tracer-test/core-test/pom.xml
✅ Files skipped from review due to trivial changes (2)
- sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/test/java/com/sofa/tracer/plugins/datasource/base/BaseTest.java
- tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/AbstractTestBase.java
🔇 Additional comments (12)
tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/SpringBootWebApplication.java (2)
20-20: LGTM!The import statement for
EmbeddedMongoAutoConfigurationis correctly added to support the configuration exclusion.
32-36: LGTM! Good practice to exclude unnecessary auto-configuration.The exclusion of
EmbeddedMongoAutoConfigurationaligns well with the PR objective of making theembedmongo-springdependency optional. This prevents the auto-configuration from attempting to start an embedded MongoDB instance when it's not needed for the tests, potentially improving test execution time and reducing resource usage.tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/ConfigurationHolderListener.java (2)
36-36: LGTM! Method usage correctly updated.The
SofaBootEnvUtils.isSpringCloudBootstrapEnvironment()call has been properly updated to use the new utility class while maintaining the same functionality.
21-21: LGTM! Verify complete migration of utility classes.The import change aligns with the PR objective of migrating from
infra-sofa-boot-startertosofa-boot.Let's verify no other references to the old package exist:
✅ Verification successful
Migration to
SofaBootEnvUtilsis complete and consistentThe verification confirms:
- No references to the old
sofa.infra.utils.SOFABootEnvUtilspackage exist- The new
sofa.boot.util.SofaBootEnvUtilsis properly used in both test and main code- No remaining references to
infra-sofa-boot-starterin build files🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old SOFABootEnvUtils package # Expect: No results, indicating complete migration rg "com\.alipay\.sofa\.infra\.utils\.SOFABootEnvUtils" --type javaLength of output: 68
Script:
#!/bin/bash # Let's verify with a broader search to ensure complete migration # 1. Check for any references to old package rg "sofa\.infra\.utils\.SOFABootEnvUtils" --type java # 2. Check current usage of SofaBootEnvUtils to confirm proper migration rg "sofa\.boot\.util\.SofaBootEnvUtils" --type java # 3. Check for any remaining references to infra-sofa-boot-starter in build files fd "pom.xml|build.gradle" --exec grep -l "infra-sofa-boot-starter" {}Length of output: 501
tracer-sofa-boot-starter/src/test/java/com/alipay/sofa/tracer/boot/base/AbstractTestCloudBase.java (2)
44-44: LGTM! Good practice making the base test class abstract.Making
AbstractTestCloudBaseabstract is a good practice as:
- It enforces the class's role as a base test class
- Prevents direct instantiation
- Matches the "Abstract" prefix in the class name
21-21: Verify the SofaBootstrapRunListener import change.The import statement has been updated to use the new package location
com.alipay.sofa.boot.listener, which aligns with the PR objective of switching to thesofa-bootdependency where this class was migrated.Let's verify the class exists in the new location:
✅ Verification successful
Let me try a different search strategy to verify the import change.
Let me try one final verification to check the dependency in the module's pom.xml specifically.
Import change verified and confirmed correct
The verification confirms:
- The
sofa-bootdependency is correctly added in the module's pom.xml- The new import
com.alipay.sofa.boot.listener.SofaBootstrapRunListeneris used consistently in test files- No references to the old package
com.alipay.sofa.infra.listenerexist in the codebase🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify SofaBootstrapRunListener exists in the new package # and is not present in the old package echo "Checking new package location..." ast-grep --pattern 'package com.alipay.sofa.boot.listener; $$$ class SofaBootstrapRunListener { $$$ }' echo "Checking old package location (should not exist)..." ast-grep --pattern 'package com.alipay.sofa.infra.listener; $$$ class SofaBootstrapRunListener { $$$ }'Length of output: 402
Script:
#!/bin/bash # Search for any file containing SofaBootstrapRunListener class definition echo "Searching for SofaBootstrapRunListener class definition..." rg "class SofaBootstrapRunListener" -A 2 # Search for any imports of SofaBootstrapRunListener echo -e "\nSearching for imports of SofaBootstrapRunListener..." rg "import.*SofaBootstrapRunListener" # Check dependencies in pom.xml echo -e "\nChecking sofa-boot dependency in pom.xml..." rg -A 3 "sofa-boot" "pom.xml"Length of output: 970
Script:
#!/bin/bash # Check dependencies in the module's pom.xml echo "Checking dependencies in tracer-sofa-boot-starter/pom.xml..." cat tracer-sofa-boot-starter/pom.xml | grep -A 5 -B 5 "sofa-boot" # Also check if there are any references to the old package echo -e "\nChecking for any references to old package..." rg "com.alipay.sofa.infra.listener"Length of output: 1010
tracer-sofa-boot-starter/src/main/java/com/alipay/sofa/tracer/boot/listener/SofaTracerConfigurationListener.java (2)
21-21: LGTM: Import change aligns with dependency updates.The addition of
SofaBootEnvUtilsimport is consistent with the PR's objective to utilize functionality from the newly addedsofa-bootdependency.
48-50: Verify behavior consistency with SofaBootEnvUtils.The change to use
SofaBootEnvUtils.isSpringCloudBootstrapEnvironmentimproves code maintainability by centralizing the environment check logic. However, we should verify that the behavior remains consistent with the previous implementation.tracer-sofa-boot-starter/pom.xml (3)
20-23: LGTM: Addition of sofa-boot dependency.This addition aligns with the PR objectives and is necessary to restore the
SofaBootstrapRunListenerfunctionality, which was previously available throughinfra-sofa-boot-starter.
272-273: LGTM: Making embedmongo-spring optional.Making this dependency optional is appropriate as it helps fix the compilation issues mentioned in the PR objectives and prevents unnecessary transitive dependencies for projects that don't need MongoDB testing capabilities.
275-278: Consider removing explicit junit dependency.The
junitdependency is already included transitively throughspring-boot-starter-test. Unless you need a specific version of JUnit, this explicit declaration might be redundant.Let's verify the transitive dependency:
pom.xml (1)
258-258: Verify compatibility with Mockito 4.3.0The upgrade from Mockito 1.10.19 to 4.3.0 is a significant version jump that may introduce breaking changes in test code. Please ensure:
- All test classes are compatible with Mockito 4.x API
- No deprecated methods are in use
✅ Verification successful
Mockito 4.3.0 upgrade appears safe to proceed
Based on the analysis of Mockito usage across the test files:
- No usage of deprecated
Matchersclass (replaced byArgumentMatchersin Mockito 2.x+)- No usage of deprecated
MockitoJUnitRunnerclass- No usage of
RETURNS_DEEP_STUBSwhich had behavior changes- All mock usage follows modern patterns compatible with Mockito 4.x:
- Standard
mock()method calls- Modern
when()andverify()syntax- Proper use of argument matchers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for common Mockito API usage that might need updates echo "Searching for potential compatibility issues..." # Check for RETURNS_DEEP_STUBS usage (behavior changed in 2.x) rg "RETURNS_DEEP_STUBS" -l # Check for common deprecated methods rg "org\.mockito\.Matchers" -l # replaced with org.mockito.ArgumentMatchers rg "org\.mockito\.runners\." -l # replaced with org.mockito.junit.MockitoJUnitRunner # Check mock() method usage (syntax changed in 2.x) rg "mock\((.*?),.*?withSettings\(\)" -lLength of output: 240
Script:
#!/bin/bash # Let's try a broader search for Mockito usage patterns to better assess compatibility # Look for Mockito imports and common patterns with context echo "=== Searching for Mockito usage patterns ===" rg "import.*org\.mockito" -l echo -e "\n=== Checking test files with Mockito annotations ===" rg "@Mock|@InjectMocks|@Spy" -l echo -e "\n=== Looking for specific Mockito method calls ===" # Common methods that might need updates between versions rg "verify\(|when\(|mock\(" -A 2 echo -e "\n=== Checking for test base classes and runners ===" rg "extends.*Test|@RunWith" -lLength of output: 42752
Motivation:
修复单元测试，主要修复tracer-sofa-boot-starter模块下单元测试不能正常运行的问题。
Modification:
Support springcloud 3.1.x #512 该pr中移除了infra-sofa-boot-starter依赖，这导致运行时缺少了com.alipay.sofa.boot.listener.SofaBootstrapRunListener，从而导致SofaTracerConfigurationListener无法正常的进行参数注入，测试用例无法正常执行。
项目正常依赖的sofaboot版本中，SofaBootstrapRunListener已经迁移到了com.alipay.sofa:sofa-boot中，不再需要依赖infra-sofa-boot-starter，重新将该依赖补回即可。
Result:
Fixes #.
If there is no issue then describe the changes introduced by this PR.
Summary by CodeRabbit
New Features
sofa-boot, enhancing project functionality.junitdependency to improve testing capabilities.Bug Fixes
mockito-coredependency to version4.3.0, improving testing performance and reliability.mockito-corein several plugins, allowing for the use of the latest version.Refactor
AbstractTestCloudBasefrom a concrete to an abstract class to better serve as a base for other classes.Chores