Skip to content

test(app_config) :: test the documented connection timeout defaults - #1436

Open
81reap wants to merge 1 commit into
sqlpage:mainfrom
81reap:mutation/12-1-connection-timeout-defaults
Open

test(app_config) :: test the documented connection timeout defaults#1436
81reap wants to merge 1 commit into
sqlpage:mainfrom
81reap:mutation/12-1-connection-timeout-defaults

Conversation

@81reap

@81reap 81reap commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

configuration.md documents that (1) the idle timeout defaults to 30 minutes, (2) the maximum lifetime to an hour, (3) that SQLite doesn't have those limits, and (4) that 0 disables the timeout. Adds tests for these cases.

discovered by #1396

SQLPage/configuration.md

Lines 18 to 19 in 6d32b40

| `database_connection_idle_timeout_seconds` | SQLite: None<BR> All other: 30 minutes | Automatically close database connections after this period of inactivity. Set to 0 to disable. |
| `database_connection_max_lifetime_seconds` | SQLite: None<BR> All other: 60 minutes | Always close database connections after this amount of time. Set to 0 to disable. |

`configuration.md` documents that (1) the idle timeout defaults to 30 minutes, (2) the maximum lifetime to an hour, (3) that SQLite doesn't have those limits, and (4) that `0` disables the timeout. Adds tests for these cases.
@81reap
81reap marked this pull request as ready for review September 7, 2026 05:28
@lovasoa lovasoa changed the title fix(app_config) :: test the documented connection timeout defaults tests(app_config) :: test the documented connection timeout defaults Sep 10, 2026
@lovasoa lovasoa changed the title tests(app_config) :: test the documented connection timeout defaults test(app_config) :: test the documented connection timeout defaults Sep 10, 2026

@lovasoa lovasoa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not useful imho

Comment thread src/app_config.rs
Comment on lines +882 to +899
assert_eq!(
config.database_connection_max_lifetime,
Some(Duration::from_mins(2))
);
}

#[test]
fn connection_timeouts_fall_back_to_the_documented_defaults() {
let config = config_with_timeouts(r#"{"database_url": "postgres://localhost/test"}"#);
assert_eq!(
config.database_connection_idle_timeout,
Some(Duration::from_mins(30))
);
assert_eq!(
config.database_connection_max_lifetime,
Some(Duration::from_hours(1))
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you can add tests for the zero = None logic if you want. But we should not assert the default configuration values. These tests just add weight and have zero bug-catching power. They do not prevent the docs and actual values from drifting, they just add toil when changing defaults.

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.

2 participants