Repository navigation
Detect driver lifecycle setup and teardown methods - #13
Conversation
shossain786
left a comment
shossain786
left a comment
There was a problem hiding this comment.
Thank you @PUSHPAK-JAISWAL. The TestNG, JUnit 4 and JUnit 5 fixtures are what #5 asked for, and the checkout builds and passes. Two changes before merge.
1. Common lifecycle annotations are missing.
The set only has BeforeMethod, AfterMethod, BeforeClass, Before, After, BeforeEach and AfterEach. The most common teardown shape, @AfterClass calling driver.quit(), is not detected, and neither are @BeforeAll / @AfterAll (JUnit 5), @BeforeTest or @BeforeSuite / @AfterSuite. Please add those and a fixture for at least @AfterClass and @BeforeAll/@AfterAll.
2. The AUTO status can overpromise.
The advice says to delete the lifecycle glue. If the method also does login, navigation, cookies or data setup, deleting it loses behaviour. Please mark the finding AUTO only when every statement in the body is driver creation, driver assignment or quit. Otherwise mark it MANUAL with advice like Remove the driver setup; keep the rest. A test for a mixed method would cover this.
A nit while you are there: .quit() matches any receiver, so please limit it to a call on the driver field or local variable if that is easy.
|
@shossain786 ok i see the changes requested working on it |
|
@shossain786 i have added common life cycle annotations. |
shossain786
left a comment
shossain786
left a comment
There was a problem hiding this comment.
Thank you @PUSHPAK-JAISWAL, this covers what I asked for. The missing lifecycle annotations are added, mixed methods are now MANUAL with the "keep the rest" advice, quit is limited to the driver variable, and there are tests for each. The branch builds and the tests pass. Approved.
Nit, non-blocking: the new helper methods in Analyzer are indented inconsistently. Please fix that when you rebase.
Once #12 is merged, this PR will have a conflict in AnalyzerTest.java (the imports). Please rebase onto main and resolve it, and I'll merge right after.
|
@PUSHPAK-JAISWAL #12 is now merged into main, so this branch has a conflict in |
|
@shossain786 done you may go ahead a merge now. |
Analyzer.java used MethodDeclaration in three places without importing it; main failed to compile after #13 merged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Description
Adds MIG-017 to detect WebDriver construction or
.quit()calls in TestNG@BeforeMethod,@AfterMethod, and@BeforeClassmethods; JUnit 4@Beforeand@Aftermethods; and JUnit 5@BeforeEachand@AfterEachmethods.Findings recommend deleting the lifecycle glue and extending
BaseTest, with advice aligned to the Selenium Boot migration guide. Lifecycle methods without driver setup or teardown are ignored.Adds TestNG, JUnit 4, and JUnit 5 fixtures and documents the rule in the README.
Validation
closes #5