Skip to content

Conversation

@agentpietrucha
Copy link
Contributor

@agentpietrucha agentpietrucha commented Nov 10, 2025

Ticket

JIRA-VPN-4403

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Checklist:

  • Changelog

Screenshots (optional, if UI related)


This change is Reviewable

@agentpietrucha agentpietrucha self-assigned this Nov 10, 2025
@agentpietrucha agentpietrucha changed the base branch from develop to vpn-4401-jwt-skew-cache November 10, 2025 15:14
@agentpietrucha agentpietrucha changed the base branch from vpn-4401-jwt-skew-cache to develop November 10, 2025 15:15
@agentpietrucha
Copy link
Contributor Author

This PR is built on top of the #3878 for better readability

@agentpietrucha agentpietrucha marked this pull request as ready for review November 10, 2025 17:14
Copy link
Collaborator

@pronebird pronebird left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @mmsinclair, @neacsu, and @trojanfoe)


nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 176 at r1 (raw file):

    pub async fn get_remote_time(&self) -> Result<VpnApiTime> {
        #[cfg(test)]
        if let Some(mocked) = self.mock_remote_time.read().await.clone() {

This looks odd. Instead we should look into mocking api client

@agentpietrucha
Copy link
Contributor Author

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @mmsinclair, @neacsu, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 176 at r1 (raw file):

    pub async fn get_remote_time(&self) -> Result<VpnApiTime> {
        #[cfg(test)]
        if let Some(mocked) = self.mock_remote_time.read().await.clone() {

This looks odd. Instead we should look into mocking api client

Could you elaborate more? Maybe with a little example?

@agentpietrucha
Copy link
Contributor Author

I also about using httpmock::MockServer to mock the health response. Something like this:

server.mock(|when, then| {
    when.method(GET).path("/api/public/v1/health");
    then.status(200)
        .header("content-type", "application/json")
        .body(format!(r#"{{"status":"ok","timestampUtc":"{timestampUtc}"}}"#));
});

Do you prefer the MockServer approach?

@agentpietrucha agentpietrucha mentioned this pull request Nov 13, 2025
1 task
Copy link
Contributor

@trojanfoe trojanfoe left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @mmsinclair, @neacsu, and @pronebird)


nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 176 at r1 (raw file):

Previously, agentpietrucha (Karol Sawicki) wrote…

Could you elaborate more? Maybe with a little example?

What Andrej is referring to is Dependency Injection, using the mocked object during testing and the real thing elsewhere.

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.

4 participants