diff --git a/README.md b/README.md index fbde405..1a07e15 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,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` | The Selenium Boot [getting-started guide](https://docs.seleniumboot.com/docs/getting-started) documents page objects extending `BasePage`, with a `WebDriver` constructor and `By` locator fields. It does not document `@FindBy` or `PageFactory.initElements`; the analyzer therefore reports their counts for review rather than treating them as a direct `BasePage` mapping. diff --git a/src/main/java/com/seleniumboot/migrator/Analyzer.java b/src/main/java/com/seleniumboot/migrator/Analyzer.java index 26906d2..f452a58 100644 --- a/src/main/java/com/seleniumboot/migrator/Analyzer.java +++ b/src/main/java/com/seleniumboot/migrator/Analyzer.java @@ -9,13 +9,21 @@ 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; 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 +32,10 @@ /** 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", "AfterClass", "BeforeTest", "AfterTest", + "BeforeSuite", "AfterSuite", "Before", "After", "BeforeEach", "AfterEach", "BeforeAll", "AfterAll"); + private final JavaParser parser = new JavaParser( new ParserConfiguration().setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17)); @@ -135,6 +147,65 @@ 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 -> { + 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() + "()", + 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) { + 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(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()) + .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 bcb546d..822bf0a 100644 --- a/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java +++ b/src/test/java/com/seleniumboot/migrator/AnalyzerTest.java @@ -32,6 +32,58 @@ 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(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."))); + } + + @Test + void detectsJunit4DriverLifecycleMethods() throws Exception { + var findings = new Analyzer().analyze(fixture("lifecycle-junit4")).findings().stream() + .filter(finding -> finding.ruleId().equals("MIG-017")).toList(); + assertEquals(3, 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(4, 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 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 new file mode 100644 index 0000000..b7161cb --- /dev/null +++ b/src/test/resources/lifecycle-junit4/BaseTest.java @@ -0,0 +1,18 @@ +class BaseTest { + WebDriver driver; + + @Before + void setUp() { + driver = new ChromeDriver(); + } + + @After + 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 new file mode 100644 index 0000000..e3732fb --- /dev/null +++ b/src/test/resources/lifecycle-junit5/BaseTest.java @@ -0,0 +1,23 @@ +class BaseTest { + WebDriver driver; + + @BeforeAll + static void setUpAll() { + driver = new ChromeDriver(); + } + + @BeforeEach + void setUp() { + driver = new ChromeDriver(); + } + + @AfterEach + 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 new file mode 100644 index 0000000..288bee9 --- /dev/null +++ b/src/test/resources/lifecycle-testng/BaseTest.java @@ -0,0 +1,38 @@ +class BaseTest { + WebDriver driver; + + @BeforeMethod + void setUp() { + driver = new ChromeDriver(); + } + + @AfterMethod + void tearDown() { + driver.quit(); + } + + @BeforeClass + 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