Add Unique Id for all test class instances #3083
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis update introduces a significant shift in how TestNG handles test class objects, focusing on enhancing object management and internal workings. By associating unique IDs with test class instances and adopting Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
testng-core/src/test/resources/testng.xmlis excluded by:!**/*.xml
Files selected for processing (26)
- CHANGES.txt (1 hunks)
- testng-core-api/src/main/java/org/testng/IClass.java (2 hunks)
- testng-core-api/src/main/java/org/testng/IObject.java (1 hunks)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/TestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (6 hunks)
- testng-core/src/main/java/org/testng/internal/ClassImpl.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11 hunks)
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1 hunks)
- testng-core/src/test/java/test/factory/ObjectIdTest.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files not reviewed due to errors (1)
- (no review received)
Additional comments: 87
testng-core-api/src/main/java/org/testng/IClass.java (4)
- 7-7: The extension of
IClassbyIObjectaligns with the PR's objective to associate unique IDs with test class instances. This architectural change facilitates the management of test class instances and their unique identification.- 30-32: Deprecating
getInstances(boolean create)in favor of a new method that includes an error message prefix is a good practice. It enhances error reporting by providing more context when instances cannot be created or retrieved.- 35-35: Introducing
getInstances(boolean create, String errorMsgPrefix)as a default method ensures backward compatibility while encouraging the use of the new method signature for enhanced error context. This is a thoughtful addition.- 40-40: Deprecating
addInstance(Object instance)without introducing a new method to replace it directly in this interface might indicate that instance management is being centralized elsewhere, possibly within theIObjectinterface or its implementations. Ensure that this change aligns with the overall architecture and does not leave a gap in functionality.Verification successful
The search results confirm that instance management is being centralized in the
IObjectinterface and its implementations, as indicated by the presence of theaddObject(IdentifiableObject instance)method. This suggests that the deprecation ofaddInstance(Object instance)in theIClassinterface aligns with an architectural shift towards centralizing instance management, and there does not appear to be a gap in functionality. Therefore, the review comment is consistent with the changes in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that instance management is adequately handled elsewhere in the codebase. rg "addObject\(IdentifiableObject instance\)" --type javaLength of output: 766
testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (2)
- 5-5: The addition of the
org.testng.IObjectimport is necessary for the changes made in this file, specifically for usingIObject.IdentifiableObjectin method signatures. This change is consistent with the PR's objectives.- 36-36: Changing the
instanceparameter type in thefindOrCreateIClassmethod fromObjecttoIObject.IdentifiableObjectis a key part of implementing the unique ID feature for test class instances. This ensures that every instance managed byBaseClassFinderis identifiable, supporting the overall goal of improved instance management and garbage collection.testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (2)
- 3-3: The addition of the
org.testng.IObjectimport is necessary for the changes made in this file, specifically for usingIObject.IdentifiableObjectin the test method. This change is consistent with the PR's objectives.- 40-40: Modifying the argument passed to
IdentifiableObjectin thefindDependedUponMethodsmethod to use an instance of the test class aligns with the goal of associating unique IDs with test class instances. This ensures that the test infrastructure itself is compatible with the new unique ID feature.testng-core/src/test/java/test/factory/ObjectIdTest.java (2)
- 18-28: The test
ensureOnlyOneObjectIdExistsForNormalTestClasscorrectly verifies that a unique ID is associated with each instance ofSampleTestCase. This test ensures that the implementation of unique IDs works as expected for standard test classes.- 30-40: The test
ensureOnlyOneObjectIdExistsForFactoryPoweredTestClassverifies that a unique ID is associated with each instance created by a factory, with the expected number of unique IDs matching the number of instances created. This test ensures that the implementation of unique IDs works as expected for factory-powered test classes.testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1)
- 17-66: The
SampleTestCaseclass correctly implements the recording of instances and their unique IDs in a static map. This setup is crucial for verifying that unique IDs are properly associated with each test class instance. The use ofConcurrentHashMapforobjectMapensures thread safety, which is important given the parallel execution of tests.testng-core-api/src/main/java/org/testng/IObject.java (1)
- 7-64: The introduction of the
IObjectinterface and theIdentifiableObjectnested class is a key part of the PR's objective to associate unique IDs with test class instances. The design allows for the encapsulation of test class instances with their unique IDs, facilitating better instance management and garbage collection. The implementation ofequalsandhashCodemethods inIdentifiableObjectbased on the instance ensures correct behavior in collections.testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1)
- 19-65: The
FactoryTestCaseclass correctly implements a factory method and the recording of instances and their unique IDs in a static map. This setup is crucial for verifying that unique IDs are properly associated with each factory-powered test class instance. The use ofConcurrentHashMapforobjectMapensures thread safety, which is important given the parallel execution of tests and the potential for concurrent modifications to the map.testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1)
- 64-64: Replacing the lambda expression with an instance of
IdentifiableObjectinitialized with the lambda expression inGuiceHelperTestensures that the unique ID feature is integrated with Guice injection. This change demonstrates the adaptability of the unique ID feature to different parts of the TestNG framework, including dependency injection.testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (2)
- 100-108: The addition of methods to handle
IdentifiableObjectinstances inFakeTestClassensures that the unique ID feature can be tested in a controlled environment. These methods are crucial for verifying the behavior of the unique ID feature in various scenarios.- 118-119: The
addObject(IdentifiableObject instance)method inFakeTestClassallows for the addition of identifiable instances to the test class, supporting the testing of instance management with unique IDs. This method complements thegetObjectsmethods by providing a way to populate the test class with instances for testing.testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (3)
- 25-25: Changing the type of
m_instancestoIdentifiableObject[]inNoOpTestClassis a necessary update to align with the unique ID feature. This change ensures that even no-operation test classes can handle identifiable instances, maintaining consistency across the framework.- 137-140: The introduction of the
getObjects(boolean create)method returningIdentifiableObject[]ensures thatNoOpTestClasscan provide identifiable instances, aligning with the unique ID feature's requirements. This method supports the consistent handling of instances across different parts of the TestNG framework.- 155-156: The
addObject(IdentifiableObject instance)method allows for the addition of identifiable instances toNoOpTestClass, supporting the framework's ability to manage instances with unique IDs consistently, even in no-operation contexts.testng-core/src/main/java/org/testng/internal/ClassImpl.java (10)
- 3-3: Importing
java.util.Arraysis a good addition given the use ofArrays.streamin thegetInstancesmethod to work with the newIdentifiableObjecttype.- 27-27: Changing the type of
m_defaultInstancefromObjecttoIdentifiableObjectaligns with the PR's objective to enhance object management within TestNG by associating unique IDs with test class objects.- 29-29: Renaming
m_instancestoidentifiableObjectsand changing its type to a list ofIdentifiableObjectis a logical step following the introduction ofIdentifiableObject. This change improves clarity and aligns with the new approach.- 32-32: The change in the type of
m_instancetoIdentifiableObjectis consistent with the overall goal of the PR. It ensures that each test class instance is wrapped with an object that includes a unique ID.- 53-54: The use of
IdentifiableObject.unwrap(instance)to check if the instance is of typeITestis a good practice. It ensures that the unique ID functionality is transparent to existing logic that expects plainObjectinstances.- 104-115: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [94-110]
The
getDefaultInstancemethod's adaptation to work withIdentifiableObjectis well-implemented. Wrapping the raw object withIdentifiableObjectwhen creating a default instance ensures consistency with the new object management approach.
- 123-126: Converting
IdentifiableObjectinstances back toObjectingetInstancesusingArrays.streamandmapis a neat way to maintain backward compatibility with existing code that expectsObject[].- 129-131: The
addObjectmethod correctly adds anIdentifiableObjectinstance to theidentifiableObjectslist, ensuring that all test class instances are managed with their unique IDs.- 133-149: The
getObjectsmethod's logic to returnIdentifiableObject[]and updatem_instanceHashCodesbased on the instances' hash codes is correctly implemented. It ensures that the unique ID functionality is integrated seamlessly with existing features that rely on instance hash codes.- 161-161: The
addInstancemethod's adaptation to wrap the providedObjectinstance withIdentifiableObjectbefore adding it to the list is a necessary change to align with the new object management strategy.testng-core/src/main/java/org/testng/internal/TestNGMethod.java (4)
- 9-9: The import of
org.testng.IObjectis necessary for the file to useIObject.IdentifiableObject, aligning with the PR's objective to manage test class instances with unique IDs.- 37-37: The constructor of
TestNGMethodnow correctly expects anIObject.IdentifiableObjectinstead of a plainObject. This change is crucial for integrating the unique ID functionality into the method handling logic.- 47-47: The private constructor's adaptation to accept
IObject.IdentifiableObjectis consistent with the changes made to the public constructor, ensuring that all instances handled byTestNGMethodare associated with unique IDs.- 175-175: The
clonemethod's modification to create a newIdentifiableObjectwith the instance and its ID ensures that the clonedTestNGMethodretains the unique ID association of the original instance. This is a critical aspect of maintaining the integrity of the unique ID functionality across method clones.testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1)
- 181-181: The modification in
createListenerFactoryto access the instance viaic.getObjects(false)[0].getInstance()is necessary to adapt to the use ofIdentifiableObject. This ensures that the listener factory is instantiated with the correct object instance, aligning with the new object management strategy.testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2)
- 7-7: The import of
org.testng.IObjectis necessary for the file to useIObject.IdentifiableObject, aligning with the PR's objective to manage test class instances with unique IDs.- 191-195: The adaptation in
computeParametersto useIObject.IdentifiableObjectand access the instance viagetInstance()is a necessary change to align with the new object management strategy. This ensures that the correct instance is used when instantiating nested classes.testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2)
- 6-6: The import of
java.util.UUIDis correctly added to support the generation of unique IDs for test instances. This aligns with the PR's objective to enhance instance management within TestNG.- 368-371: The implementation of
getInstanceId()method inWrappedTestNGMethodcorrectly delegates the call to the wrappedtestNGMethodinstance. This ensures that the unique ID functionality is properly propagated through the proxy pattern. However, it's important to ensure that thetestNGMethodinstance itself supports this new method and returns a meaningfulUUID.Verification successful
Given the lack of output from the initial script, it's necessary to refine our approach to verify the presence of the
getInstanceIdmethod in classes implementingITestNGMethod. Let's start by identifying any classes that implementITestNGMethodand then check for thegetInstanceIdmethod within those classes.
Given the repeated lack of output from our attempts to directly find classes implementing
ITestNGMethodand those with agetInstanceIdmethod, it's necessary to verify the existence of theITestNGMethodinterface itself within the codebase. This step will help confirm that we are searching within the correct context and that the interface is indeed part of the codebase.
Given the confirmed existence of the
ITestNGMethodinterface within the codebase, the next step is to broaden our search approach to identify any potential usage or implementation of thegetInstanceIdmethod. This will help in understanding how and where this method is supported across the codebase.
The search across the codebase has confirmed the presence of the
getInstanceIdmethod in both theITestNGMethodinterface and its various implementations, includingTestNGMethod.java,ConfigurationMethod.java,BaseTestMethod.java, and others. This supports the review comment's assertion that thegetInstanceIdmethod is correctly delegated to thetestNGMethodinstance inWrappedTestNGMethod, ensuring that the unique ID functionality is properly propagated through the proxy pattern. Based on these findings, the implementation ofgetInstanceId()inWrappedTestNGMethodis verified to be in alignment with the expected behavior.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the testNGMethod instance (likely implementations of ITestNGMethod) supports getInstanceId method and returns a valid UUID. ast-grep --lang java --pattern $'class $_ implements ITestNGMethod { $$$ getInstanceId() { $$$ } $$$ }'Length of output: 104
Script:
#!/bin/bash # First, find any classes implementing ITestNGMethod ast-grep --lang java --pattern $'class $_ implements ITestNGMethod { $$$ }'Length of output: 76
Script:
#!/bin/bash # Verify the existence of ITestNGMethod interface in the codebase rg "interface ITestNGMethod" -lLength of output: 93
Script:
#!/bin/bash # Search for the getInstanceId method across the entire codebase rg "getInstanceId" -lLength of output: 623
testng-core-api/src/main/java/org/testng/ITestNGMethod.java (2)
- 6-6: The import of
java.util.UUIDis correctly added to support the newgetInstanceId()method, which is essential for generating unique IDs for test instances.- 301-307: The addition of the
getInstanceId()method in theITestNGMethodinterface is a crucial part of the PR's objective to associate unique IDs with test class objects. The default implementation returningnullis a safe placeholder, but concrete implementations must override this method to return a meaningfulUUID. It's important to ensure all implementations ofITestNGMethodprovide a valid override of this method.testng-core/src/main/java/org/testng/internal/FactoryMethod.java (4)
- 15-15: The import of
org.testng.IObjectis correctly added to support the use ofIObject.IdentifiableObjectwithinFactoryMethod. This aligns with the PR's objective to enhance instance management by associating unique IDs with test class objects.- 75-80: The modification of the
FactoryMethodconstructor to accept anIObject.IdentifiableObjectinstead of a plainObjectinstance is a key part of integrating the unique ID functionality. This change ensures that factory methods can work with identifiable test instances, supporting the overall goal of improved instance management.- 82-82: The use of
IObject.IdentifiableObject.unwrap(identifiable)to extract the actual instance from theIdentifiableObjectwrapper is correct and necessary for backward compatibility with existing code that expects plainObjectinstances. This approach maintains the integrity of the unique ID feature while ensuring minimal disruption to existing functionalities.- 122-122: The assignment of
m_instanceusinggetInstance()is a subtle yet important change. It ensures that the instance variable withinFactoryMethodcorrectly references the unwrapped test instance. This change is crucial for maintaining the expected behavior of factory methods while incorporating the new unique ID feature.testng-core/src/main/java/org/testng/TestClass.java (6)
- 109-115: The update to use
IdentifiableObjectinstances and the subsequent stream operations to handle test instances are correctly implemented. This change is crucial for integrating the unique ID functionality into theTestClassand ensuring that test names can be derived fromITestinstances when available. This approach enhances the flexibility and robustness of test instance management.- 126-129: The addition of the
getObjectsmethod override inTestClassto return an array ofIdentifiableObjectis a necessary update to support the new unique ID feature. This method ensures that all parts of the TestNG framework can work with identifiable test instances, facilitating better instance management and garbage collection.- 136-139: Similar to the previous comment, the addition of the
getObjectsmethod override with anerrorMsgPrefixparameter is correctly implemented. This ensures consistency in how identifiable test instances are retrieved throughout the TestNG framework, aligning with the PR's objectives.- 151-154: The implementation of the
addObjectmethod inTestClassto accept anIdentifiableObjectinstance is a key part of integrating the unique ID functionality. This change allows for the addition of identifiable test instances to the internal structures of TestNG, supporting the overall goal of improved instance management.- 148-163: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [160-199]
The modifications to use
IdentifiableObjectinstances in the initialization of configuration methods are correctly implemented. These changes ensure that all configuration methods can work with identifiable test instances, which is essential for the unique ID feature to function as intended across the TestNG framework.
- 251-251: The update to iterate over
IdentifiableObjectinstances when creating test methods is correctly implemented. This ensures that test methods are associated with identifiable test instances, aligning with the PR's objective to improve test instance management through the use of unique IDs.testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (2)
- 10-10: The addition of the
org.testng.IObjectimport is necessary for the new functionality introduced in this PR. It enables the use ofIObject.IdentifiableObjectwithin the test methods.- 237-237: The modification to wrap the
objectwithnew IObject.IdentifiableObject(object)is essential for associating unique IDs with test class objects. This change aligns with the PR's objective to improve internal handling and garbage collection of test class instances. Ensure that this change does not inadvertently affect the functionality of existing tests.testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (3)
- 9-11: The addition of imports for
java.util.Objectsandjava.util.UUIDsupports the new functionality of generating unique keys for locking mechanisms. These are necessary for the enhancements made in this PR.- 52-52: The introduction of a static final field
lockof typeKeyAwareAutoCloseableLockis crucial for the new locking mechanism based on unique keys. Ensure that this locking mechanism is thread-safe and does not introduce deadlocks.- 126-128: The implementation of the locking mechanism using
UUIDfor generating unique keys andKeyAwareAutoCloseableLockfor managing locks is well-designed. It ensures that each test method instance is handled uniquely, which is crucial for the parallel execution of tests. Using the try-with-resources statement for automatic lock release is a good practice. However, it's important to thoroughly test this implementation to identify any potential issues related to locking, such as deadlocks or performance impacts.Also applies to: 137-137
testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (11)
- 17-17: The import of
org.testng.IObjectis correctly added to support the new functionality of associating unique IDs with test class objects.- 38-38: The change from
ObjecttoIObject.IdentifiableObjectin the declaration ofm_instanceMapis crucial for associating unique IDs with test class objects. This aligns with the PR's objectives and enhances the handling of test class instances.- 51-51: The method signature of
TestNGClassFinderconstructor now acceptsMap<Class<?>, List<IObject.IdentifiableObject>>instead of a map with plainObjectinstances. This change is necessary for the integration of unique IDs with test class objects and ensures type safety.- 75-80: The loop iterating over
m_instanceMapand callingic.addObject(instance)for eachIdentifiableObjectensures that all test class instances are correctly associated with their unique IDs. This is a key part of the implementation for managing test class instances more effectively.- 88-88: The method signature of
processClasshas been correctly updated to acceptMap<Class<?>, List<IObject.IdentifiableObject>>as an argument. This change is consistent with the overall goal of associating unique IDs with test class objects.- 100-101: The retrieval and handling of
IdentifiableObjectinstances frominstanceMapwithinprocessClassmethod are correctly implemented. This ensures that each test class instance is managed with its unique ID.- 169-171: The method
processFactorycorrectly retrievesIdentifiableObjectinstances usingic.getObjects(false). This is part of the mechanism to ensure that factory methods work with the new system of unique IDs for test class instances.- 184-196: The loop in
processFactorymethod that handles instances returned by factory methods is correctly updated to work withIdentifiableObjectandIInstanceInfo. This ensures that both types of objects are properly managed with their unique IDs.- 330-330: The method
addInstance(IInstanceInfo<T> ii)is correctly updated to wrap the instance withIObject.IdentifiableObject. This is essential for associating unique IDs with instances that are added to the test context.- 333-338: The method
addInstance(IObject.IdentifiableObject o)correctly determines the class of the instance, taking into account whether it is an instance ofIParameterInfo. This ensures that the correct class is used when adding instances with unique IDs.- 343-345: The method
addInstance(Class<S> clazz, IObject.IdentifiableObject instance)correctly addsIdentifiableObjectinstances tom_instanceMap. This is a key part of managing test class instances with unique IDs.testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11)
- 8-9: The addition of import statements for
OptionalandIObjectaligns with the changes made in the rest of the file to support the new functionality of associating unique IDs with test class instances. This is a necessary step to ensure that the new types are available for use within the file.- 71-71: Changing the parameter type from
ObjecttoIObject.IdentifiableObjectin the constructor signature is a key part of implementing the unique ID functionality. This change ensures that each test class instance can be associated with a unique ID, facilitating better internal handling and potentially improving garbage collection efficiency.- 126-126: The change in the parameter type for another constructor of
ConfigurationMethodis consistent with the overall goal of associating unique IDs with test class instances. This ensures that all instances created or manipulated by this class are properly wrapped with their unique IDs.- 160-160: The modification in the
createMethodsstatic method to acceptIObject.IdentifiableObjectinstead ofObjectis crucial for ensuring that all test methods created by this method are associated with their unique IDs. This change is consistent with the PR's objective and is necessary for the correct functioning of the new feature.- 200-200: The change in the parameter type for
createSuiteConfigurationMethodsis aligned with the changes in other parts of the file and is necessary for the implementation of the unique ID functionality. This ensures that suite configuration methods are also associated with unique IDs.- 224-224: Similarly, the change in the parameter type for
createTestConfigurationMethodssupports the PR's objective by ensuring that test configuration methods are associated with unique IDs. This is consistent with the changes made in other parts of the file.- 247-247: The modification in the parameter type for
createClassConfigurationMethodsis necessary for the implementation of the unique ID functionality, ensuring that class configuration methods are associated with unique IDs. This change is consistent with the overall goal of the PR.- 269-269: The change in the parameter type for
createBeforeConfigurationMethodsaligns with the PR's objective by ensuring that before configuration methods are associated with unique IDs. This is consistent with the changes made in other parts of the file.- 300-300: The modification in the parameter type for
createAfterConfigurationMethodssupports the PR's objective by ensuring that after configuration methods are associated with unique IDs. This is consistent with the changes made in other parts of the file.- 331-331: The change in the parameter type for
createTestMethodConfigurationMethodsis aligned with the changes in other parts of the file and is necessary for the implementation of the unique ID functionality. This ensures that test method configuration methods are also associated with unique IDs.- 511-511: The cloning method has been updated to ensure that the cloned
ConfigurationMethodinstance is associated with the same unique ID as the original instance. This is a crucial change for maintaining the integrity of the unique ID functionality across cloned instances.testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (8)
- 12-12: The import of
java.util.UUIDis correctly added to support the generation or handling of unique IDs within the class.- 19-19: The import of
org.testng.IObjectis necessary for the changes made in this file, specifically for using theIObject.IdentifiableObjecttype.- 86-86: The addition of the
m_instancefield of typeIObject.IdentifiableObjectis crucial for associating a unique ID with each test class instance. This change aligns with the PR's objective to improve garbage collection efficiency by making test class instances more identifiable.- 96-96: The constructor now accepts an
IObject.IdentifiableObjectinstance as a parameter. This modification is necessary to initialize them_instancefield with a unique ID wrapped test class instance. It's important to ensure that all invocations of this constructor throughout the codebase are updated to pass the requiredIdentifiableObject.- 153-157: The
getInstance()method has been updated to return the actual test class instance from theIdentifiableObject. This change is necessary to maintain compatibility with existing code that expects an instance of the test class. The use ofOptionaland method chaining ensures null safety, which is a good practice.- 159-161: The addition of the
getInstanceId()method is a direct consequence of the PR's objective to associate unique IDs with test class instances. This method allows for the retrieval of the unique ID associated with the test class instance, which is essential for the improved garbage collection mechanism.- 392-393: The modification to the
hashCode()method to include the identity hash code of the instance obtained viagetInstance()is a thoughtful addition. It ensures that the hash code considers the unique identity of the test class instance, which is crucial for using these instances as keys in maps or collections.- 803-807: The
getFactoryMethodParamsInfo()method now checks if the instance wrapped byIdentifiableObjectimplementsIParameterInfo. This change is necessary to support scenarios where test class instances are created with parameters, and it's important to retrieve these parameters for further processing or reporting.testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2)
- 30-30: The addition of the
IObjectimport is necessary for implementing the unique ID functionality for test class instances. This change aligns with the PR's objectives.- 385-392: The modification to use
IObject.IdentifiableObjectfor handling test class instances and accessing the actual instance withinstance.getInstance()is a key part of integrating the unique ID functionality. This change ensures that the unique ID is properly associated with each test class instance, which is crucial for the objectives of this PR. However, it's important to ensure that all parts of the code that interact with test class instances are updated to accommodate this new approach to avoid any potential issues.
- Ensure that the rest of the codebase that interacts with test class instances is aware of this change and correctly handles
IdentifiableObjectinstances.- Consider the performance implications of adding an additional layer of abstraction with
IdentifiableObject. While it's unlikely to have a significant impact, it's worth keeping in mind, especially in large test suites.Verification successful
The modification to use
IObject.IdentifiableObjectfor handling test class instances, as seen inTestInvoker.javaand also applied inNoOpTestClass.java, indicates a consistent application of this approach across the codebase. This supports the integration of the unique ID functionality and suggests that relevant parts of the system have been updated to accommodate this change. It remains important to ensure that all interactions with test class instances are correctly handled and to consider the performance implications of these changes, especially in large test suites.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all interactions with test class instances have been updated. rg "getInstances\(true\)" --type java # Check for potential performance implications in large test suites. echo "Manual review recommended to assess performance impact in large test suites."Length of output: 202
Script:
#!/bin/bash # Corrected verification to search for the actual method usage "getObjects(true)". rg "getObjects\(true\)" --type java # Reiterate the recommendation for a manual review to assess performance impact in large test suites. echo "Manual review recommended to assess performance impact in large test suites."Length of output: 456
testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
Outdated
Show resolved
Hide resolved
testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
testng-core/src/test/resources/testng.xmlis excluded by:!**/*.xml
Files selected for processing (26)
- CHANGES.txt (1 hunks)
- testng-core-api/src/main/java/org/testng/IClass.java (2 hunks)
- testng-core-api/src/main/java/org/testng/IObject.java (1 hunks)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/TestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (6 hunks)
- testng-core/src/main/java/org/testng/internal/ClassImpl.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11 hunks)
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1 hunks)
- testng-core/src/test/java/test/factory/ObjectIdTest.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (25)
- CHANGES.txt
- testng-core-api/src/main/java/org/testng/IClass.java
- testng-core-api/src/main/java/org/testng/IObject.java
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/TestClass.java
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/ClassImpl.java
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java
- testng-core/src/test/java/test/factory/ObjectIdTest.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
Additional comments: 2
testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2)
- 6-6: The addition of
java.util.UUIDimport is appropriate for generating unique IDs, aligning with the PR's objective to associate unique IDs with test class objects. This import is necessary for the implementation of thegetInstanceId()method.- 3-9: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-22]
The use of
java.util.Randomfor generating amultiplicationFactoris noted. While this usage is not directly related to security contexts such as password or token generation, it's important to be aware of the limitations ofRandomfor cryptographic purposes. In this specific case, the use ofRandomis acceptable as it's used to generate a hashcode multiplier, not for security-sensitive operations.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
testng-core/src/test/resources/testng.xmlis excluded by:!**/*.xml
Files selected for processing (26)
- CHANGES.txt (1 hunks)
- testng-core-api/src/main/java/org/testng/IClass.java (2 hunks)
- testng-core-api/src/main/java/org/testng/IObject.java (1 hunks)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/TestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (6 hunks)
- testng-core/src/main/java/org/testng/internal/ClassImpl.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11 hunks)
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1 hunks)
- testng-core/src/test/java/test/factory/ObjectIdTest.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (25)
- CHANGES.txt
- testng-core-api/src/main/java/org/testng/IClass.java
- testng-core-api/src/main/java/org/testng/IObject.java
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/TestClass.java
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/ClassImpl.java
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java
- testng-core/src/test/java/test/factory/ObjectIdTest.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
Additional comments: 3
testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (3)
- 6-6: The import of
UUIDis correctly added to support the generation of unique IDs for test instances. This aligns with the PR's objective to enhance instance management within TestNG.- 368-371: The implementation of
getInstanceId()correctly delegates the call to the wrappedITestNGMethodinstance. This approach is consistent with the proxy pattern used inWrappedTestNGMethod. Ensure that theITestNGMethodinterface and its implementations are updated to include thegetInstanceId()method, as this change impacts the contract ofITestNGMethod.- 3-9: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-22]
The use of
Randomto generate amultiplicationFactorfor thehashCode()method is noted. While the static analysis tool suggests usingSecureRandomfor cryptographic strength, the current usage context does not involve security-sensitive operations. However, it's important to ensure that the randomness provided byRandomis sufficient for the intended purpose of creating a uniquehashCode. If the uniqueness ofhashCodeis critical, consider evaluating the distribution and collision probability of the generated values.
juherr
reviewed
Mar 7, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
testng-core/src/test/resources/testng.xmlis excluded by:!**/*.xml
Files selected for processing (26)
- CHANGES.txt (1 hunks)
- testng-core-api/src/main/java/org/testng/IClass.java (1 hunks)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (3 hunks)
- testng-core-api/src/main/java/org/testng/internal/IObject.java (1 hunks)
- testng-core/src/main/java/org/testng/TestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (6 hunks)
- testng-core/src/main/java/org/testng/internal/ClassImpl.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11 hunks)
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (7 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1 hunks)
- testng-core/src/test/java/test/factory/ObjectIdTest.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (24)
- CHANGES.txt
- testng-core-api/src/main/java/org/testng/IClass.java
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/TestClass.java
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/ClassImpl.java
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java
- testng-core/src/test/java/test/factory/ObjectIdTest.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
Additional comments: 10
testng-core-api/src/main/java/org/testng/internal/IObject.java (8)
- 22-22: The method
getObjectsis well-defined, providing a mechanism to retrieve instances associated with a class, optionally creating them if necessary. This method supports the core functionality of managing test instances with unique IDs.- 25-25: The method
getInstanceHashCodesoffers a way to retrieve hash codes of instances, which could be useful for debugging or logging purposes. However, ensure that the use of hash codes aligns with the unique ID mechanism and does not introduce any potential for confusion or collision.- 28-28: The
addObjectmethod allows adding a new instance to the list of managed instances. This is crucial for dynamically managing instances, especially in scenarios involving factories or dynamically generated tests.- 34-36: The static method
instanceHashCodesprovides a utility to retrieve hash codes for instances that are compatible withIObject. This method enhances the interface's usability by allowing operations on generic objects without explicit casting by the caller.- 44-46: The overloaded static method
objectswithout theerrorMsgPrefixparameter simplifies the retrieval ofIdentifiableObjectarrays by providing a default error message prefix. This method enhances usability by reducing the need for callers to specify common parameters.- 56-60: The static method
objectswith theerrorMsgPrefixparameter is a comprehensive way to retrieveIdentifiableObjectarrays, allowing for customized error handling. This method is essential for scenarios where detailed error messages are necessary for debugging or logging.- 67-72: The static method
castprovides a safe way to cast objects toIObjectif they implement the interface. This utility method is crucial for working with objects in a type-safe manner, reducing the risk ofClassCastException.- 75-115: The nested class
IdentifiableObjectis well-designed, encapsulating a test class instance and its associated unique ID. The constructors, accessors, and overriddenequalsandhashCodemethods are correctly implemented, ensuring that instances are uniquely identifiable and can be used effectively in collections. This class is central to the enhancement introduced by this pull request, enabling efficient management and identification of test class instances.testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2)
- 6-6: The import of
java.util.UUIDis necessary for the new functionality introduced by thegetInstanceIdmethod. This addition is correctly placed and follows Java import conventions.- 368-371: The implementation of the
getInstanceIdmethod correctly delegates the call to the wrappedITestNGMethodinstance. This approach maintains consistency with the proxy pattern used throughout theWrappedTestNGMethodclass. However, ensure that theITestNGMethodinterface and its implementations are updated to include thegetInstanceId()method, as this change impacts the contract ofITestNGMethod.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
testng-core/src/test/resources/testng.xmlis excluded by:!**/*.xml
Files selected for processing (26)
- CHANGES.txt (1 hunks)
- testng-core-api/src/main/java/org/testng/IClass.java (1 hunks)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/TestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (6 hunks)
- testng-core/src/main/java/org/testng/internal/ClassImpl.java (8 hunks)
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java (11 hunks)
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/IObject.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java (5 hunks)
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java (7 hunks)
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java (2 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java (1 hunks)
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java (2 hunks)
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java (1 hunks)
- testng-core/src/test/java/test/factory/ObjectIdTest.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (24)
- CHANGES.txt
- testng-core-api/src/main/java/org/testng/IClass.java
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/TestClass.java
- testng-core/src/main/java/org/testng/internal/BaseClassFinder.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/ClassImpl.java
- testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java
- testng-core/src/main/java/org/testng/internal/FactoryMethod.java
- testng-core/src/main/java/org/testng/internal/NoOpTestClass.java
- testng-core/src/main/java/org/testng/internal/TestListenerHelper.java
- testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java
- testng-core/src/main/java/org/testng/internal/TestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/main/java/org/testng/internal/objects/SimpleObjectDispenser.java
- testng-core/src/test/java/org/testng/internal/DynamicGraphHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodHelperTest.java
- testng-core/src/test/java/org/testng/internal/MethodInstanceTest.java
- testng-core/src/test/java/org/testng/internal/dynamicgraph/FakeTestClass.java
- testng-core/src/test/java/org/testng/internal/objects/GuiceHelperTest.java
- testng-core/src/test/java/test/factory/ObjectIdTest.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
juherr
requested changes
Mar 13, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (7)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (7 hunks)
- testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (7 hunks)
- testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
juherr
approved these changes
Mar 14, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java (7 hunks)
- testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java (1 hunks)
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java (3 hunks)
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java (4 hunks)
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java (1 hunks)
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- testng-core-api/src/main/java/org/testng/ITestNGMethod.java
- testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
- testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java
- testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java
- testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java
- testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java
- testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3079
Fixes #3079 .
Did you remember to?
CHANGES.txt./gradlew autostyleApplyWe encourage pull requests that:
If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.
Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.
Summary by CodeRabbit
DataProviderbehavior to align with previous versions for retrying tests.synchronizedblocks withReentrantLockfor improved concurrency handling.IdentifiableObjectandUUID.