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

registry_auth::token_not_logged is failing randomly #12639

Closed
ehuss opened this issue Sep 7, 2023 · 3 comments
Closed

registry_auth::token_not_logged is failing randomly #12639

ehuss opened this issue Sep 7, 2023 · 3 comments
Assignees
Labels
A-testing-cargo-itself Area: cargo's tests

Comments

@ehuss
Copy link
Contributor

ehuss commented Sep 7, 2023

The test registry_auth::token_not_logged has been failing in CI randomly, and I don't immediately have thoughts why.

---- registry_auth::token_not_logged stdout ----
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: None,
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/3/b/bar",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/dl/bar/1.0.0/download",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
/home/runner/work/cargo/cargo/target/tmp/cit/t2285/dl/bar/1.0.0/download
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/api/v1/crates/new",
        query: None,
        fragment: None,
    },
    method: "put",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/config.json",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
req: Request {
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                127.0.0.1,
            ),
        ),
        port: Some(
            37477,
        ),
        path: "/index/3/f/foo",
        query: None,
        fragment: None,
    },
    method: "get",
    authorization: Some(
        "a-unique_token",
    ),
    if_modified_since: None,
    if_none_match: None,
}
thread 'registry_auth::token_not_logged' panicked at tests/testsuite/registry_auth.rs:499:5:
assertion `left == right` failed
  left: 8
 right: 7

This issue has been assigned to @hi-rustin via this comment.

@ehuss ehuss added the A-testing-cargo-itself Area: cargo's tests label Sep 7, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Sep 7, 2023

@rustbot assign @hi-rustin

@hi-rustin indicated they may be able to look at this at #12624 (comment). Let me know if you have any questions or want me to take a look to give you any pointers.

@rustbot rustbot self-assigned this Sep 7, 2023
bors added a commit that referenced this issue Sep 22, 2023
Fix spurious errors with networking tests.

This fixes an issue where some networking tests could behave erratically. In particular, the `registry_auth::token_not_logged` has been failing somewhat often (see #12639). The issue is that curl can behave inconsistently based on whether or not it immediately detects that the connection has closed or not, which is not done consistently. HTTP 1.1 defaults to `Connection: open`, so this mini HTTP server was essentially not standards compliant. `Connection: close` tells curl to expect the connection to close ([ref](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection)).
@ehuss
Copy link
Contributor Author

ehuss commented Sep 22, 2023

@hi-rustin I'm not sure if you've had a chance to look at this, but since it has been a little disruptive I went ahead and posted #12726 and #12727 to fix this. There seemed to be two independent errors, hence the two PRs.

bors added a commit that referenced this issue Sep 23, 2023
Buffer console status messages.

This adds buffering to some of the console output. This can help with interleaved output, particularly with things like the log output interleaving with status messages. This fixes that interleaving by atomically writing the entire status message, which in turn, helps fix some spurious errors such as #12639.

I'm uncertain what the performance impact of this change might have. It might improve performance, since there should be a lot fewer syscalls, and I suspect terminals will be able to handle it more efficiently (and particularly across a network connection). However, I don't know if that will have a noticeable impact.

Only the "status" messages are buffered. Everything else is still unbuffered as before.
@ehuss
Copy link
Contributor Author

ehuss commented Oct 19, 2023

Closing as this should now be resolved by #12726 and #12727.

@ehuss ehuss closed this as completed Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing-cargo-itself Area: cargo's tests
Projects
None yet
Development

No branches or pull requests

2 participants