Skip to content

Conversation

@stevebio
Copy link
Collaborator

Fix SSL tests by exposing port 8017 in docker-compose files and adding new matching error message text in the SSL to non-SSL REST service test assert call.

…or port 8017. Add a second error message for test using SSL to confirm error when connecting to non-SSL REST service. Message probably changed for ML12 (not sure, but error message in test does not work).
Copilot AI review requested due to automatic review settings September 15, 2025 22:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fix SSL-related test failures by updating docker port configurations and improving error message handling. The changes ensure SSL tests can properly connect to port 8017 and handle different error message formats that may occur during SSL-to-HTTP connection attempts.

  • Exposed port 8017 in docker-compose configurations to enable SSL test connectivity
  • Enhanced error message matching in SSL tests to handle multiple possible error formats
  • Updated both standard and nightly docker-compose files for consistency

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test-basic/client.js Enhanced error message assertion to handle both configuration error and protocol error messages
test-app/docker-compose.yaml Extended port range from 8015-8016 to 8015-8017 to include SSL test port
test-app/docker-compose-nightlies.yaml Extended port range from 8000-8016 to 8000-8017 to include SSL test port

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +149 to +150
assert(error.message.toString().includes('You have attempted to access an HTTP server using HTTPS. Please check your configuration.') ||
error.message.toString().includes('write EPROTO'));
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message checking logic is duplicated with error.message.toString() called twice. Consider storing the result in a variable to improve readability and avoid redundant computation.

Suggested change
assert(error.message.toString().includes('You have attempted to access an HTTP server using HTTPS. Please check your configuration.') ||
error.message.toString().includes('write EPROTO'));
const errorMsg = error.message.toString();
assert(errorMsg.includes('You have attempted to access an HTTP server using HTTPS. Please check your configuration.') ||
errorMsg.includes('write EPROTO'));

Copilot uses AI. Check for mistakes.
@anu3990
Copy link
Contributor

anu3990 commented Sep 15, 2025

Looks good. Please merge if the Jenkins PR-build succeeds. Thanks.!

@stevebio stevebio merged commit 963cba1 into develop Sep 16, 2025
2 checks passed
@anu3990 anu3990 deleted the task/fixSslTests branch September 16, 2025 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants