From c156bfcf1f5c93c3af357b196deb999926a3c939 Mon Sep 17 00:00:00 2001 From: Pushpak-Jaiswal Date: Sat, 26 Sep 2026 11:57:36 +0530 Subject: [PATCH 1/2] feat: threadLock-driver setup in TestNG --- README.md | 1 + .../com/seleniumboot/migrator/Analyzer.java | 28 ++++++++++++++ .../seleniumboot/migrator/AnalyzerTest.java | 37 +++++++++++++++++++ .../resources/lifecycle-junit4/BaseTest.java | 13 +++++++ .../resources/lifecycle-junit5/BaseTest.java | 13 +++++++ .../resources/lifecycle-testng/BaseTest.java | 18 +++++++++ 6 files changed, 110 insertions(+) create mode 100644 src/test/resources/lifecycle-junit4/BaseTest.java create mode 100644 src/test/resources/lifecycle-junit5/BaseTest.java create mode 100644 src/test/resources/lifecycle-testng/BaseTest.java diff --git a/README.md b/README.md index 0342637..56523c2 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ Each rule follows the [Selenium + TestNG migration guide](https://docs.seleniumb | MIG-014 | `Thread.sleep` | Manual review | | MIG-015 | Custom `*DriverManager` / `*DriverFactory` | Manual review | | MIG-016 | `implicitlyWait` | Remove; manual review | +| MIG-017 | Driver creation or quit in TestNG/JUnit lifecycle methods | Delete lifecycle glue; extend `BaseTest` | ## Adding a rule diff --git a/src/main/java/com/seleniumboot/migrator/Analyzer.java b/src/main/java/com/seleniumboot/migrator/Analyzer.java index e6594dc..47dfe4a 100644 --- a/src/main/java/com/seleniumboot/migrator/Analyzer.java +++ b/src/main/java/com/seleniumboot/migrator/Analyzer.java @@ -16,6 +16,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.stream.Stream; import static com.seleniumboot.migrator.Finding.Status.AUTO; @@ -24,6 +25,9 @@ /** Read-only static analysis: parses Java sources and reports patterns that map onto Selenium Boot. */ public final class Analyzer { + private static final Set DRIVER_LIFECYCLE_ANNOTATIONS = Set.of( + "BeforeMethod", "AfterMethod", "BeforeClass", "Before", "After", "BeforeEach", "AfterEach"); + private final JavaParser parser = new JavaParser( new ParserConfiguration().setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17)); @@ -115,6 +119,30 @@ private void scan(CompilationUnit cu, String file, List out) { .filter(c -> c.getNameAsString().endsWith("DriverManager") || c.getNameAsString().endsWith("DriverFactory")) .forEach(c -> out.add(new Finding("MIG-015", MANUAL, file, line(c), c.getNameAsString(), "Custom driver lifecycle: review, then replace with BaseTest."))); + // MIG-017: driver lifecycle managed in TestNG or JUnit setup/teardown methods + cu.findAll(MethodDeclaration.class).stream() + .filter(this::isDriverLifecycleMethod) + .forEach(method -> out.add(new Finding("MIG-017", AUTO, file, line(method), + "Driver setup/teardown in @" + lifecycleAnnotation(method) + " " + method.getNameAsString() + "()", + "Delete the lifecycle glue; extend BaseTest. Driver creation, per-thread isolation, and teardown are handled for you."))); + } + + private boolean isDriverLifecycleMethod(MethodDeclaration method) { + if (lifecycleAnnotation(method) == null || method.getBody().isEmpty()) return false; + var body = method.getBody().get(); + boolean createsDriver = body.findAll(ObjectCreationExpr.class).stream() + .anyMatch(creation -> creation.getType().getNameAsString().endsWith("Driver")); + boolean quitsDriver = body.findAll(MethodCallExpr.class).stream() + .anyMatch(call -> call.getNameAsString().equals("quit")); + return createsDriver || quitsDriver; + } + + private String lifecycleAnnotation(MethodDeclaration method) { + return method.getAnnotations().stream() + .map(annotation -> annotation.getName().getIdentifier()) + .filter(DRIVER_LIFECYCLE_ANNOTATIONS::contains) + .findFirst() + .orElse(null); } private static int line(Node n) { diff --git a/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java b/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java index 0c24e49..06fb4f7 100644 --- a/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java +++ b/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java @@ -2,12 +2,18 @@ import org.junit.jupiter.api.Test; +import java.net.URISyntaxException; +import java.nio.file.Path; import java.util.List; import static org.junit.jupiter.api.Assertions.*; class AnalyzerTest { + private static Path fixture(String name) throws URISyntaxException { + return Path.of(AnalyzerTest.class.getResource("/" + name).toURI()); + } + private static List rules(String src) { return new Analyzer().analyzeSource(src).findings().stream().map(Finding::ruleId).toList(); } @@ -24,6 +30,37 @@ public class DriverFactory { assertTrue(r.contains("MIG-015")); } + @Test + void detectsTestNgDriverLifecycleMethods() throws Exception { + var findings = new Analyzer().analyze(fixture("lifecycle-testng")).findings().stream() + .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); + assertEquals(3, findings.size()); + assertTrue(findings.stream().allMatch(finding -> finding.advice().contains( + "Delete the lifecycle glue; extend BaseTest. Driver creation, per-thread isolation, and teardown are handled for you."))); + } + + @Test + void detectsJunit4DriverLifecycleMethods() throws Exception { + var findings = new Analyzer().analyze(fixture("lifecycle-junit4")).findings().stream() + .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); + assertEquals(2, findings.size()); + assertTrue(findings.stream().allMatch(finding -> finding.advice().contains("extend BaseTest"))); + } + + @Test + void detectsJunit5DriverLifecycleMethods() throws Exception { + var findings = new Analyzer().analyze(fixture("lifecycle-junit5")).findings().stream() + .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); + assertEquals(2, findings.size()); + assertTrue(findings.stream().allMatch(finding -> finding.advice().contains("extend BaseTest"))); + } + + @Test + void ignoresLifecycleMethodsWithoutDriverSetupOrTeardown() { + var report = new Analyzer().analyzeSource("class BaseTest { @BeforeEach void setUp() { prepareData(); } }"); + assertFalse(report.findings().stream().anyMatch(finding -> finding.ruleId().equals("MIG-017"))); + } + @Test void detectsWaitsSleepsAndImplicitWait() { var r = rules(""" diff --git a/src/test/resources/lifecycle-junit4/BaseTest.java b/src/test/resources/lifecycle-junit4/BaseTest.java new file mode 100644 index 0000000..a6a38c7 --- /dev/null +++ b/src/test/resources/lifecycle-junit4/BaseTest.java @@ -0,0 +1,13 @@ +class BaseTest { + WebDriver driver; + + @Before + void setUp() { + driver = new ChromeDriver(); + } + + @After + void tearDown() { + driver.quit(); + } +} \ No newline at end of file diff --git a/src/test/resources/lifecycle-junit5/BaseTest.java b/src/test/resources/lifecycle-junit5/BaseTest.java new file mode 100644 index 0000000..4d03e67 --- /dev/null +++ b/src/test/resources/lifecycle-junit5/BaseTest.java @@ -0,0 +1,13 @@ +class BaseTest { + WebDriver driver; + + @BeforeEach + void setUp() { + driver = new ChromeDriver(); + } + + @AfterEach + void tearDown() { + driver.quit(); + } +} \ No newline at end of file diff --git a/src/test/resources/lifecycle-testng/BaseTest.java b/src/test/resources/lifecycle-testng/BaseTest.java new file mode 100644 index 0000000..2366803 --- /dev/null +++ b/src/test/resources/lifecycle-testng/BaseTest.java @@ -0,0 +1,18 @@ +class BaseTest { + WebDriver driver; + + @BeforeMethod + void setUp() { + driver = new ChromeDriver(); + } + + @AfterMethod + void tearDown() { + driver.quit(); + } + + @BeforeClass + void setUpClass() { + driver = new FirefoxDriver(); + } +} \ No newline at end of file From 62aa3762e90841a16d0d49d9ee4efe8ad5f179eb Mon Sep 17 00:00:00 2001 From: Pushpak-Jaiswal Date: Sat, 26 Sep 2026 21:46:36 +0530 Subject: [PATCH 2/2] requested updates and tests --- .../com/seleniumboot/migrator/Analyzer.java | 51 +++++++++++++++++-- .../seleniumboot/migrator/AnalyzerTest.java | 27 ++++++++-- .../resources/lifecycle-junit4/BaseTest.java | 5 ++ .../resources/lifecycle-junit5/BaseTest.java | 10 ++++ .../resources/lifecycle-testng/BaseTest.java | 20 ++++++++ 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/seleniumboot/migrator/Analyzer.java b/src/main/java/com/seleniumboot/migrator/Analyzer.java index 47dfe4a..18b69e9 100644 --- a/src/main/java/com/seleniumboot/migrator/Analyzer.java +++ b/src/main/java/com/seleniumboot/migrator/Analyzer.java @@ -9,7 +9,14 @@ import com.github.javaparser.ast.expr.MethodCallExpr; import com.github.javaparser.ast.expr.NameExpr; import com.github.javaparser.ast.expr.ObjectCreationExpr; +import com.github.javaparser.ast.expr.AssignExpr; +import com.github.javaparser.ast.expr.Expression; +import com.github.javaparser.ast.expr.FieldAccessExpr; +import com.github.javaparser.ast.expr.ThisExpr; import com.github.javaparser.ast.Node; +import com.github.javaparser.ast.stmt.ExpressionStmt; +import com.github.javaparser.ast.stmt.Statement; +import com.github.javaparser.ast.expr.VariableDeclarationExpr; import java.io.IOException; import java.nio.file.Files; @@ -26,7 +33,8 @@ public final class Analyzer { private static final Set DRIVER_LIFECYCLE_ANNOTATIONS = Set.of( - "BeforeMethod", "AfterMethod", "BeforeClass", "Before", "After", "BeforeEach", "AfterEach"); + "BeforeMethod", "AfterMethod", "BeforeClass", "AfterClass", "BeforeTest", "AfterTest", + "BeforeSuite", "AfterSuite", "Before", "After", "BeforeEach", "AfterEach", "BeforeAll", "AfterAll"); private final JavaParser parser = new JavaParser( new ParserConfiguration().setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17)); @@ -122,9 +130,15 @@ private void scan(CompilationUnit cu, String file, List out) { // MIG-017: driver lifecycle managed in TestNG or JUnit setup/teardown methods cu.findAll(MethodDeclaration.class).stream() .filter(this::isDriverLifecycleMethod) - .forEach(method -> out.add(new Finding("MIG-017", AUTO, file, line(method), + .forEach(method -> { + boolean lifecycleOnly = method.getBody().orElseThrow().getStatements().stream() + .allMatch(this::isLifecycleGlueStatement); + out.add(new Finding("MIG-017", lifecycleOnly ? AUTO : MANUAL, file, line(method), "Driver setup/teardown in @" + lifecycleAnnotation(method) + " " + method.getNameAsString() + "()", - "Delete the lifecycle glue; extend BaseTest. Driver creation, per-thread isolation, and teardown are handled for you."))); + lifecycleOnly + ? "Delete the lifecycle glue; extend BaseTest. Driver creation, per-thread isolation, and teardown are handled for you." + : "Remove the driver setup; keep the rest.")); + }); } private boolean isDriverLifecycleMethod(MethodDeclaration method) { @@ -133,10 +147,39 @@ private boolean isDriverLifecycleMethod(MethodDeclaration method) { boolean createsDriver = body.findAll(ObjectCreationExpr.class).stream() .anyMatch(creation -> creation.getType().getNameAsString().endsWith("Driver")); boolean quitsDriver = body.findAll(MethodCallExpr.class).stream() - .anyMatch(call -> call.getNameAsString().equals("quit")); + .anyMatch(this::isDriverQuit); return createsDriver || quitsDriver; } + private boolean isLifecycleGlueStatement(Statement statement) { + if (!(statement instanceof ExpressionStmt expressionStatement)) return false; + Expression expression = expressionStatement.getExpression(); + if (expression instanceof MethodCallExpr call) return isDriverQuit(call); + if (expression instanceof AssignExpr assignment) return isDriverVariable(assignment.getTarget()); + if (expression instanceof VariableDeclarationExpr declaration) { + return declaration.getVariables().stream().allMatch(variable -> + variable.getNameAsString().equals("driver") + && variable.getInitializer().filter(ObjectCreationExpr.class::isInstance) + .map(ObjectCreationExpr.class::cast) + .map(creation -> creation.getType().getNameAsString().endsWith("Driver")) + .orElse(false)); + } + return expression instanceof ObjectCreationExpr creation + && creation.getType().getNameAsString().endsWith("Driver"); + } + + private boolean isDriverQuit(MethodCallExpr call) { + return call.getNameAsString().equals("quit") + && call.getScope().filter(this::isDriverVariable).isPresent(); + } + + private boolean isDriverVariable(Expression expression) { + if (expression instanceof NameExpr name) return name.getNameAsString().equals("driver"); + return expression instanceof FieldAccessExpr field + && field.getNameAsString().equals("driver") + && field.getScope() instanceof ThisExpr; + } + private String lifecycleAnnotation(MethodDeclaration method) { return method.getAnnotations().stream() .map(annotation -> annotation.getName().getIdentifier()) diff --git a/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java b/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java index 06fb4f7..8898f33 100644 --- a/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java +++ b/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java @@ -34,7 +34,7 @@ public class DriverFactory { void detectsTestNgDriverLifecycleMethods() throws Exception { var findings = new Analyzer().analyze(fixture("lifecycle-testng")).findings().stream() .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); - assertEquals(3, findings.size()); + assertEquals(7, findings.size()); assertTrue(findings.stream().allMatch(finding -> finding.advice().contains( "Delete the lifecycle glue; extend BaseTest. Driver creation, per-thread isolation, and teardown are handled for you."))); } @@ -43,7 +43,7 @@ void detectsTestNgDriverLifecycleMethods() throws Exception { void detectsJunit4DriverLifecycleMethods() throws Exception { var findings = new Analyzer().analyze(fixture("lifecycle-junit4")).findings().stream() .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); - assertEquals(2, findings.size()); + assertEquals(3, findings.size()); assertTrue(findings.stream().allMatch(finding -> finding.advice().contains("extend BaseTest"))); } @@ -51,7 +51,7 @@ void detectsJunit4DriverLifecycleMethods() throws Exception { void detectsJunit5DriverLifecycleMethods() throws Exception { var findings = new Analyzer().analyze(fixture("lifecycle-junit5")).findings().stream() .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); - assertEquals(2, findings.size()); + assertEquals(4, findings.size()); assertTrue(findings.stream().allMatch(finding -> finding.advice().contains("extend BaseTest"))); } @@ -61,6 +61,27 @@ void ignoresLifecycleMethodsWithoutDriverSetupOrTeardown() { assertFalse(report.findings().stream().anyMatch(finding -> finding.ruleId().equals("MIG-017"))); } + @Test + void mixedLifecycleMethodRequiresManualReview() { + var finding = new Analyzer().analyzeSource(""" + class BaseTest { + @AfterEach void tearDown() { + driver.quit(); + logout(); + } + } + """).findings().stream().filter(item -> item.ruleId().equals("MIG-017")).findFirst().orElseThrow(); + + assertEquals(Finding.Status.MANUAL, finding.status()); + assertEquals("Remove the driver setup; keep the rest.", finding.advice()); + } + + @Test + void ignoresQuitOnNonDriverReceiver() { + var report = new Analyzer().analyzeSource("class BaseTest { @AfterEach void tearDown() { browser.quit(); } }"); + assertFalse(report.findings().stream().anyMatch(finding -> finding.ruleId().equals("MIG-017"))); + } + @Test void detectsWaitsSleepsAndImplicitWait() { var r = rules(""" diff --git a/src/test/resources/lifecycle-junit4/BaseTest.java b/src/test/resources/lifecycle-junit4/BaseTest.java index a6a38c7..b7161cb 100644 --- a/src/test/resources/lifecycle-junit4/BaseTest.java +++ b/src/test/resources/lifecycle-junit4/BaseTest.java @@ -10,4 +10,9 @@ void setUp() { void tearDown() { driver.quit(); } + + @AfterClass + void tearDownClass() { + driver.quit(); + } } \ No newline at end of file diff --git a/src/test/resources/lifecycle-junit5/BaseTest.java b/src/test/resources/lifecycle-junit5/BaseTest.java index 4d03e67..e3732fb 100644 --- a/src/test/resources/lifecycle-junit5/BaseTest.java +++ b/src/test/resources/lifecycle-junit5/BaseTest.java @@ -1,6 +1,11 @@ class BaseTest { WebDriver driver; + @BeforeAll + static void setUpAll() { + driver = new ChromeDriver(); + } + @BeforeEach void setUp() { driver = new ChromeDriver(); @@ -10,4 +15,9 @@ void setUp() { void tearDown() { driver.quit(); } + + @AfterAll + static void tearDownAll() { + driver.quit(); + } } \ No newline at end of file diff --git a/src/test/resources/lifecycle-testng/BaseTest.java b/src/test/resources/lifecycle-testng/BaseTest.java index 2366803..288bee9 100644 --- a/src/test/resources/lifecycle-testng/BaseTest.java +++ b/src/test/resources/lifecycle-testng/BaseTest.java @@ -15,4 +15,24 @@ void tearDown() { void setUpClass() { driver = new FirefoxDriver(); } + + @BeforeTest + void setUpTest() { + driver = new ChromeDriver(); + } + + @AfterTest + void tearDownTest() { + driver.quit(); + } + + @BeforeSuite + void setUpSuite() { + driver = new ChromeDriver(); + } + + @AfterSuite + void tearDownSuite() { + driver.quit(); + } } \ No newline at end of file