[java] Fix "secure vs non-secure" error in tests

[asolntsev]

After switching "non-secure" test environment to "secure", the global GlobalTestEnvironment still continued holding the previous "non-secure" instance. And the following tests failed because this instance had been stopped already.

💥 What does this PR do?

Fixes tests which are broken when I run them locally in my IDE.

🔧 Implementation Notes

See commit messages.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Type

(Describe updated until commit b9723de)

Bug fix, Tests

---

Description

• Fix test environment switching between secure and non-secure modes

• Add context switching to prevent test isolation issues

• Update form submission tests to navigate after alert handling

• Improve test cleanup and window management

---

File Walkthrough

	Relevant files
Tests	3 files AlertsTest.java Update form submission test to navigate after alert            +7/-2      FormHandlingTest.java Add navigation assertions after form submission alert        +3/-0      form_handling_js_submit.html Change form action to navigate instead of JavaScript alert +2/-2
Bug fix	3 files GlobalTestEnvironment.java Implement secure server mode detection and restart logic  +10/-4    JavaScriptTestSuite.java Simplify test environment initialization with new API        +2/-3      JupiterTestBase.java Add window context switching and improve test isolation    +26/-19
Enhancement	3 files TestEnvironment.java Add isSecure method to check server security mode                +4/-0      AppServer.java Add isSecure method to AppServer interface                              +2/-0      NettyAppServer.java Implement isSecure method to check HTTPS configuration      +4/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured warning log: The new warning log line is plain-text (non-structured) and may not meet environments requiring structured logging formats even though it appears non-sensitive. Referred Code public static synchronized TestEnvironment getOrCreate(boolean needsSecureServer) { if (needsSecureServer && environment != null && !environment.isSecure()) { LOG.log(Level.WARNING, "Restarting appServer with secureServer=true"); environment.stop(); environment = null; Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consider starting both servers always To improve efficiency, consider always initializing the test environment with both HTTP and HTTPS servers. This would prevent the overhead of restarting the environment when switching between secure and non-secure test contexts. Examples: java/test/org/openqa/selenium/environment/GlobalTestEnvironment.java [37-48] public static synchronized TestEnvironment getOrCreate(boolean needsSecureServer) { if (needsSecureServer && environment != null && !environment.isSecure()) { LOG.log(Level.WARNING, "Restarting appServer with secureServer=true"); environment.stop(); environment = null; } if (environment == null) { environment = new InProcessTestEnvironment(needsSecureServer); environment.assertIsValid(); } ... (clipped 2 lines) java/test/org/openqa/selenium/testing/JupiterTestBase.java [62-69] public void prepareEnvironment() { boolean needsSecureServer = Optional.ofNullable(this.getClass().getAnnotation(NeedsSecureServer.class)) .map(NeedsSecureServer::value) .orElse(false); environment = GlobalTestEnvironment.getOrCreate(needsSecureServer); appServer = environment.getAppServer(); Solution Walkthrough: Before: // in GlobalTestEnvironment.java public static synchronized TestEnvironment getOrCreate(boolean needsSecureServer) { if (needsSecureServer && environment != null && !environment.isSecure()) { LOG.log(Level.WARNING, "Restarting appServer with secureServer=true"); environment.stop(); environment = null; } if (environment == null) { environment = new InProcessTestEnvironment(needsSecureServer); environment.assertIsValid(); } return environment; } After: // in GlobalTestEnvironment.java public static synchronized TestEnvironment getOrCreate() { if (environment == null) { // InProcessTestEnvironment would be modified to always start both servers environment = new InProcessTestEnvironment(); environment.assertIsValid(); } return environment; } // in JupiterTestBase.java public void prepareEnvironment() { // The 'needsSecureServer' logic is no longer needed here environment = GlobalTestEnvironment.getOrCreate(); appServer = environment.getAppServer(); // ... } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies an inefficiency in the PR's approach and proposes a valid, more performant architectural alternative that would improve test environment stability.	Medium
Possible issue	Restart on secure-to-non-secure switch Add logic to restart the test environment when switching from a secure to a non-secure server, mirroring the existing logic for non-secure to secure switches. java/test/org/openqa/selenium/environment/GlobalTestEnvironment.java [37-48] public static synchronized TestEnvironment getOrCreate(boolean needsSecureServer) { if (needsSecureServer && environment != null && !environment.isSecure()) { LOG.log(Level.WARNING, "Restarting appServer with secureServer=true"); + environment.stop(); + environment = null; + } else if (!needsSecureServer && environment != null && environment.isSecure()) { + LOG.log(Level.WARNING, "Restarting appServer with secureServer=false"); environment.stop(); environment = null; } if (environment == null) { environment = new InProcessTestEnvironment(needsSecureServer); environment.assertIsValid(); } return environment; } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a missing feature for symmetrically handling server restarts when switching from secure to non-secure mode, which improves test environment consistency.	Medium
General	Use boolean flag for secure state In NettyAppServer, use a dedicated boolean flag to track the server's security mode instead of checking if the secure channel object is null. java/test/org/openqa/selenium/environment/webserver/NettyAppServer.java [192-194] +private final boolean secureServer; +... +public NettyAppServer(boolean secureServer) { + this.secureServer = secureServer; + initValues(secureServer); +} public boolean isSecure() { - return secure != null; + return secureServer; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: The suggestion improves code clarity and robustness by explicitly tracking the server's security state with a boolean flag, rather than inferring it from the secure object's nullness.	Low
Update

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix onsubmit to allow submission Modify the form's onsubmit handler to alert('Tasty cheese'); return true; to ensure the form is submitted after the alert is displayed. common/src/web/form_handling_js_submit.html [24] -<form id="theForm" method="get" action="click_tests/submitted_page.html" onsubmit="return alert('Tasty cheese')"> +<form id="theForm" method="get" action="click_tests/submitted_page.html" onsubmit="alert('Tasty cheese'); return true;"> Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical bug where return alert(...) in the onsubmit handler prevents form submission, which would cause the new and updated tests in the PR to fail.	High
	Improve test teardown window switching Improve the switchToInitialWindow method by attempting to switch to any other available window if the initial window is no longer present, enhancing test teardown stability. java/test/org/openqa/selenium/testing/JupiterTestBase.java [101-118] @AfterEach final void switchToInitialWindow() { if (driver == null) { return; } if (initialWindowHandle != null) { try { driver.switchTo().window(initialWindowHandle); } catch (NoSuchWindowException | NoSuchSessionException ok) { LOG.log( Level.FINE, String.format( "The initial window has been closed in test %s: %s", seleniumExtension.currentTest(), ok)); + // The initial window is gone. Switch to any other window so that + // subsequent tests/teardown don't fail. + try { + driver.getWindowHandles().stream() + .findFirst() + .ifPresent(handle -> driver.switchTo().window(handle)); + } catch (NoSuchSessionException ignore) { + // The session is gone. + } } } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential weakness in the new teardown logic and proposes a reasonable improvement to make the test suite more robust against edge cases.	Low
More