Skip to content

Reset WebDriver and vncRecordingContainer when Selenium container stops#5116

Merged
kiview merged 2 commits intotestcontainers:masterfrom
tobiasstadler:selenium-reset-driver
May 18, 2022
Merged

Reset WebDriver and vncRecordingContainer when Selenium container stops#5116
kiview merged 2 commits intotestcontainers:masterfrom
tobiasstadler:selenium-reset-driver

Conversation

@tobiasstadler
Copy link
Copy Markdown
Contributor

@tobiasstadler tobiasstadler commented Feb 22, 2022 •
edited
Loading

Some of our tests look similar to:

void BaseTestClass {

    @Container
    private static final BrowserWebDriverContainer<?> container = new BrowserWebDriverContainer<>("...");

    ...
}

void MyTest1 extends BaseTestClass {

    @Test
    void test1() {
        ...
    }

    ...
}

void MyTest2 extends BaseTestClass {

    @Test
    void test2() {
        ...
    }

    ...
}

The selenium container is started before any test of one class are run and stopped after all test of one class are run. Currently driver is created only once in BrowserWebDriverContainer.java and when the container stops driver.quit() is called. So the second and any subsequent test class will see a "stopped" driver which will fail the tests of that class.

Right now we recreate/start/stop the selenium container manually in @BeforeAll/@AfterAll block, but it would be nice if we could reuse the container definition.

@tobiasstadler tobiasstadler force-pushed the selenium-reset-driver branch 2 times, most recently from e42b56c to c12e889 Compare February 23, 2022 14:20
@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented Mar 4, 2022

@kiview Would you please have a look?

@kiview
Copy link
Copy Markdown
Member

kiview commented Mar 4, 2022

Hey @tobiasstadler, sure thing, but can you please update the description of the PR, so that we have the context? Like, which use cases does this unblock?

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented Mar 7, 2022

@kiview I updated the description

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented Mar 17, 2022

ping

@kiview
Copy link
Copy Markdown
Member

kiview commented Mar 17, 2022

I am not sure I understand your example correctly. In case of using a JUnit-Rule or the Jupiter-Extension on a per-test-method basis, each test would instantiate a new BrowserWebDriverContainer object.

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented Mar 18, 2022

@kiview You are right, I described the wrong case. Sorry! I changed the description to actually describe or problem.

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented Apr 14, 2022

ping

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented May 6, 2022

ping

@bsideup bsideup added this to the next milestone May 17, 2022
@bsideup
Copy link
Copy Markdown
Member

bsideup commented May 17, 2022

Hey @tobiasstadler! Thanks for spotting and fixing it. Do you think you can provide a simple test that will verify that this is no longer an issue? (so that we won't introduce a regression in the future)

@kiview kiview changed the title [Selenium] Reset the WebDriver when the Selenium container stops Reset the WebDriver when the Selenium container stops May 18, 2022
@kiview kiview changed the title Reset the WebDriver when the Selenium container stops Reset WebDriver and vncRecordingContainer when Selenium container stops May 18, 2022
@kiview kiview merged commit 6dc05d3 into testcontainers:master May 18, 2022
@kiview
Copy link
Copy Markdown
Member

kiview commented May 18, 2022

Thanks a lot for the contribution @tobiasstadler, we will take care of the test ourselves 🙂

@tobiasstadler
Copy link
Copy Markdown
Contributor Author

tobiasstadler commented May 19, 2022

Thank You!

@tobiasstadler tobiasstadler deleted the selenium-reset-driver branch May 19, 2022 17:22
eddumelendez added a commit that referenced this pull request May 19, 2022
@eddumelendez
Merge branch 'master' into gh-5116
6433274
eddumelendez added a commit that referenced this pull request May 19, 2022
@eddumelendez
Merge branch 'master' into gh-5116
93b0cfc
eddumelendez added a commit that referenced this pull request May 25, 2022
@eddumelendez
Merge branch 'master' into gh-5116
eaf0e72
eddumelendez added a commit that referenced this pull request May 26, 2022
@eddumelendez
Merge branch 'master' into gh-5116
dddca96
eddumelendez added a commit that referenced this pull request May 26, 2022
@eddumelendez
Merge branch 'master' into gh-5116
35446b5
eddumelendez added a commit that referenced this pull request May 26, 2022
@eddumelendez
Merge branch 'master' into gh-5116
aa928dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bsideup bsideup bsideup approved these changes

@kiview kiview kiview approved these changes

@rnorth rnorth Awaiting requested review from rnorth

Assignees

No one assigned

Labels

modules/selenium type/bug

Projects

None yet

Milestone

1.17.2

Development

Successfully merging this pull request may close these issues.

3 participants

@tobiasstadler @kiview @bsideup