Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selenium generic autoconfiguration #27921

Closed
cdalexndr opened this issue Sep 9, 2021 · 9 comments
Closed

Selenium generic autoconfiguration #27921

cdalexndr opened this issue Sep 9, 2021 · 9 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@cdalexndr
Copy link
Contributor

cdalexndr commented Sep 9, 2021

Currently, only HtmlUnit driver is supported, and the following utility classes are not public so they cannot be used/extended by user with other drivers: WebDriverScope, WebDriverContextCustomizerFactory, WebDriverTestExecutionListener

The rhino engine used by HtmlUnit has limitations (HtmlUnit/htmlunit#232 : mozilla/rhino#652, mozilla/rhino#678 ), and with the headless option already implemented by browsers, using other drivers is a good alternative.

Autoconfigurations should be made for other drivers, maybe with the help of WebDriveManager.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 9, 2021
@philwebb
Copy link
Member

It looks like we currently apply the WebDriverScope to all org.openqa.selenium.WebDriver beans and to the MockMvcHtmlUnitDriverBuilder (see this static final). We also have MockMvcWebDriverAutoConfiguration which creates a MockMvcHtmlUnitDriverBuilder with a WebConnectionHtmlUnitDriver.

Looking at MockMvcHtmlUnitDriverBuilder.withDelegate it appears that we can't configure anything other than an HtmlUnit driver.

@cdalexndr an you provide some more details about the kinds of drivers you want to be auto-configured and how they'll be used? Perhaps we need a Spring Framework issue to update MockMvc?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Sep 10, 2021
@cdalexndr
Copy link
Contributor Author

cdalexndr commented Sep 10, 2021

Due to htmlunit rhino engine limitation, I use Firefox with gecko driver, plus WebDriverManager for easy setup:

@Bean
//WARNING: name must be same as WebDriverScope.NAME <-- cannot access WebDriverScope.NAME to reference it
public WebDriver webDriver() {
    WebDriverManager.firefoxdriver().setup();
    return new FirefoxDriver(
            new FirefoxOptions().setHeadless( true ) );
}

I also use mockito SpyBean/MockBean in my test, so I have to register spring's mockito test execution listeners:
@TestExecutionListeners({MockitoTestExecutionListener.class, ResetMocksTestExecutionListener.class})

This will cause the default test execution listeners to not be registered, among them, the required WebDriverTestExecutionListener.
Because WebDriverTestExecutionListener class is not public, I cannot add it to the above @TestExecutionListeners for my test class (without dirty hack).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 10, 2021
@philwebb
Copy link
Member

Hmm, that's interesting. The MockitoTestExecutionListener, ResetMocksTestExecutionListener and WebDriverTestExecutionListener classes should be loaded automatically via spring.factories. You shouldn't need to declare them with the @TestExecutionListeners annotation.

Do you have a small sample that you can share that shows things not working as expected?

@philwebb philwebb added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 10, 2021
@cdalexndr
Copy link
Contributor Author

cdalexndr commented Sep 10, 2021

I'm using TestNg so I extend AbstractTransactionalTestNGSpringContextTests, that uses @TestExecutionListeners without MergeMode.MERGE_WITH_DEFAULTS.

Even if I use @TestExecutionListeners(mergeMode=MergeMode.MERGE_WITH_DEFAULTS) it will be overwritten by the above base class, and it will not merge with defaults: see code. If I don't use any @TestExecutionListeners on my class, still the defaults are not loaded due to same issue.

A possible workaround would be to use @TestExecutionListeners(inheritListeners=false,mergeMode=MergeMode.MERGE_WITH_DEFAULTS) in my class, but this way I loose all AbstractTransactionalTestNGSpringContextTests's configured TestExecutionListeners, and I don't want to copy-paste them from the library.

Note, same issue with AbstractTestNGSpringContextTests.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 10, 2021
@philwebb
Copy link
Member

I’m not that familiar with the TestNg integration, perhaps @sbrannen knows more.

@cdalexndr could we still have a sample application that shows the problem? I think it would really help us.

@philwebb philwebb added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 12, 2021
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Sep 19, 2021
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Sep 26, 2021
@cdalexndr
Copy link
Contributor Author

Kinda offtopic, because the spring testng integration is designed this way so this is not a new issue.
Here's the sample repo: https://github.com/cdalexndr/spring-boot-issue-27921

@wilkinsona wilkinsona reopened this Sep 30, 2021
@wilkinsona wilkinsona added the status: waiting-for-triage An issue we've not yet triaged label Sep 30, 2021
@snicoll
Copy link
Member

snicoll commented Dec 9, 2021

OK so the sample showcases the needs of putting these classes public because of the way TestNG works so we can continue the discussion on the PR.

@snicoll snicoll closed this as completed Dec 9, 2021
@snicoll snicoll added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

5 participants