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

Enable test should_fail_with_ssl_enabled_and_bad_ssl_config #521

Open
josecelano opened this issue Nov 24, 2023 · 2 comments
Open

Enable test should_fail_with_ssl_enabled_and_bad_ssl_config #521

josecelano opened this issue Nov 24, 2023 · 2 comments
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat Quality & Assurance Relates to QA, Testing, and CI
Milestone

Comments

@josecelano
Copy link
Member

THe following test was ignored by @da2ce7 but it seems to pass.

#[tokio::test]
#[ignore]
#[should_panic = "Could not receive bind_address."]
async fn should_fail_with_ssl_enabled_and_bad_ssl_config() {
    let mut test_env = stopped_test_environment(configuration::ephemeral());

    let cfg = test_env.config_mut();

    cfg.ssl_enabled = true;
    cfg.ssl_key_path = Some("bad key path".to_string());
    cfg.ssl_cert_path = Some("bad cert path".to_string());

    test_env.start().await;
}
@josecelano josecelano added Code Cleanup / Refactoring Tidying and Making Neat Quality & Assurance Relates to QA, Testing, and CI - Developer - Torrust Improvement Experience labels Nov 24, 2023
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Dec 22, 2023
It was ignored but it's now passing. I don't know was it was ignored.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Dec 22, 2023
It was ignored but it's now passing. I don't know was it was ignored.
@josecelano josecelano linked a pull request Dec 22, 2023 that will close this issue
@josecelano
Copy link
Member Author

The test was ignored because it's failing on the GitHub runner:

failures:

---- servers::api::v1::contract::configuration::should_fail_with_ssl_enabled_and_bad_ssl_config stdout ----
---- servers::api::v1::contract::configuration::should_fail_with_ssl_enabled_and_bad_ssl_config stderr ----
panic did not contain expected string
      panic message: `"`spawn_local` called from outside of a `task::LocalSet`"`,
 expected substring: `"Could not receive bind_address."`
thread 'main' panicked at /home/runner/work/torrust-tracker/torrust-tracker/src/servers/apis/server.rs:216:9:
`spawn_local` called from outside of a `task::LocalSet`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Anyway, I don't like the final panic message: Could not receive bind_address. I think the Launcher should catch the error and return the exact reason why the service could not be launched.

This is part of the ApiServer<Stopped>::start function:

        let task = tokio::spawn(async move {
            let (bind_addr, server) = Launcher::start(&configuration, tracker, shutdown_signal(shutdown_receiver));

            addr_sender.send(bind_addr).expect("Could not return SocketAddr.");

            server.await;
        });

What is happening is Launcher::start is panicking here:

            let tls_config = RustlsConfig::from_pem_file(ssl_cert_path, ssl_key_path)
                .await
                .expect("Could not read tls cert.");

So the line addr_sender.send(bind_addr).expect("Could not return SocketAddr."); is never reached. Maybe the channel could send something like Resultt<SocketAddr, LaunchingError>.
In the case that tls config cannot be initialized we could return that error.

enum LaunchingError {
  InvalidCertOrKeyPath
}

@josecelano
Copy link
Member Author

Hi @da2ce7 after merging #553 now it's also failing locally.

@cgbosse cgbosse moved this to Maintenance in Torrust Solution Jan 8, 2024
@cgbosse cgbosse added this to the v3.0.0 milestone Jan 16, 2024
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 11, 2024
… the service

It shows a more friendly error message and it's easier to test.
@josecelano josecelano removed a link to a pull request Apr 11, 2024
@josecelano josecelano self-assigned this Apr 11, 2024
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 15, 2024
… the service

It shows a more friendly error message and it's easier to test.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 19, 2024
… the service

It shows a more friendly error message and it's easier to test.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 19, 2024
It shows a more friendly error message and it's easier to test.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 19, 2024
It shows a more friendly error message and it's easier to test.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 19, 2024
…tarts

It shows a more friendly error message and it's easier to test.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 19, 2024
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 22, 2024
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Apr 22, 2024
@josecelano josecelano modified the milestones: v3.0.0, v3.1.0 May 10, 2024
@josecelano josecelano removed their assignment Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat Quality & Assurance Relates to QA, Testing, and CI
Projects
Status: Maintenance
Development

No branches or pull requests

2 participants