feat/xdg config dir by gbrennon · Pull Request #31 · gbrennon/BitPill

gbrennon · 2026-03-08T02:28:58Z

chore(deps): add dirs crate for XDG config path resolution
feat(infrastructure): resolve data files from ~/.config/bitpill/
test(infrastructure): wire harness and fix stale dormant tests
test(infrastructure): add AppInitializer integration tests

Copilot

Pull request overview

This PR moves BitPill’s persistent JSON data files (medications, dose records, settings) to an XDG/platform config directory by resolving paths via a new AppPaths helper, and bootstraps the directory/files on startup. It also adds/rewires infrastructure integration tests with a proper tests/infrastructure.rs harness so nested infra tests compile and run.

Changes:

Add dirs dependency and introduce infrastructure::config::{AppPaths, AppInitializer} for path resolution + first-run bootstrapping.
Update infrastructure::Container::new() to resolve and initialize config files, then construct JSON repositories from those paths.
Add an infrastructure test harness and update/add infra integration tests around JSON repositories and initialization.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`Cargo.toml`	Adds `dirs` dependency for config directory resolution.
`Cargo.lock`	Locks `dirs` and transitive deps.
`justfile`	Adds an `install` recipe for global installation.
`src/infrastructure/mod.rs`	Exposes new `config` module.
`src/infrastructure/container.rs`	Resolves XDG config paths and runs initialization before wiring repositories/services.
`src/infrastructure/config/mod.rs`	Declares config submodules.
`src/infrastructure/config/app_paths.rs`	Implements path resolution (dirs + env overrides) and test helpers.
`src/infrastructure/config/app_initializer.rs`	Bootstraps config dir + default JSON files; merges default settings keys.
`tests/infrastructure.rs`	New harness to compile/run `tests/infrastructure/*.rs`.
`tests/infrastructure/app_initializer_tests.rs`	New integration tests for initializer behavior.
`tests/infrastructure/container_settings_tests.rs`	Updates container settings-service test to use `Arc::ptr_eq`.
`tests/infrastructure/json_med_repo_integration.rs`	Updates integration test to use new container access patterns.
`tests/infrastructure/json_settings_repo_integration.rs`	Updates integration test to use `settings_service` field directly.
`tests/infrastructure/json_settings_repository_tests.rs`	Uses `tempfile` for hermetic filesystem tests.
`tests/infrastructure/integration_mark_taken.rs`	Updates integration tests for dose record persistence + mark-taken behavior.
`tests/infrastructure/integration_mark_taken_handler.rs`	Updates handler integration test wiring for updated mark-taken service deps.

Copilot · 2026-03-08T02:34:53Z

src/infrastructure/config/app_paths.rs

+/// Default resolution (no env overrides):
+///   - config dir   → `~/.config/bitpill/`
+///   - medications  → `~/.config/bitpill/medications.json`
+///   - dose records → `~/.config/bitpill/dose_records.json`
+///   - settings     → `~/.config/bitpill/settings.json`


The default-path doc comment is Unix-specific (~/.config/bitpill/…), but dirs::config_dir() returns OS-specific config locations (e.g., AppData on Windows) and the fallback is a relative .config. Update the comment to reflect platform-specific resolution and the actual fallback behavior so it stays accurate.

Copilot · 2026-03-08T02:34:53Z

tests/infrastructure/app_initializer_tests.rs

+use std::fs;
+
+use bitpill::infrastructure::config::{app_initializer::AppInitializer, app_paths::AppPaths};
+use serde_json::Value;
+use tempfile::{TempDir, tempdir};


Import grouping is out of order (internal bitpill::… comes before external crates like serde_json/tempfile). To match the repo’s Rust import conventions, group as: std imports, blank line, external crate imports, blank line, then internal/crate imports.

Copilot · 2026-03-08T02:34:54Z

tests/infrastructure/container_settings_tests.rs

 #[test]
-fn get_settings_service_returns_same_reference() {
+fn settings_service_arc_is_same_instance() {
    let c = bitpill::infrastructure::container::Container::new();


This test constructs Container::new(), which now runs AppInitializer and may create/write real user config files under the OS config directory. To keep tests hermetic and avoid modifying developer/CI machines, use Container::new_with_paths(...) with a tempdir() (or another isolated path) for this test.

Copilot · 2026-03-08T02:34:54Z

tests/infrastructure/integration_mark_taken_handler.rs

+    let mut container = Container::new();
+    container.mark_dose_taken_service = Arc::new(MarkDoseTakenService::new(
+        fake_dose_repo,
+        fake_med_repo,
+    )) as Arc<dyn MarkDoseTakenPort>;


Creating a default Container::new() here will run AppInitializer and can touch the real OS config directory (creating data files) during test runs. Use Container::new_with_paths(...) with tempdir() for an isolated container, then override mark_dose_taken_service as needed.

Copilot · 2026-03-08T02:34:54Z

tests/infrastructure/integration_mark_taken.rs

+    let req = MarkDoseTakenRequest::new(med_res.id.clone(), taken_at);
+    let res = container
+        .mark_dose_taken_service
+        .execute(req)
+        .expect("marking dose as taken should succeed");

-    // Read the saved record back and assert it is taken
-    let record_id = DoseRecordId::from(uuid::Uuid::parse_str(&res.id).unwrap());
-    let saved = fake_repo
-        .find_by_id(&record_id)
-        .expect("repo call should succeed")
-        .expect("record should exist");
-
-    assert!(
-        saved.is_taken(),
-        "record saved by MarkDoseTakenService must be taken"
-    );
-    assert_eq!(saved.taken_at(), Some(taken_at));
+    assert!(!res.record_id.is_empty());


This test only asserts that a non-empty record_id is returned, but the behavior under test is that a taken record is created and persisted. Strengthen the test to verify observable persistence (e.g., read dose_records.json and assert an entry exists for res.record_id with the expected taken_at).

Copilot · 2026-03-08T02:34:54Z

src/infrastructure/config/app_initializer.rs

+    pub fn initialize(paths: &AppPaths) -> io::Result<()> {
+        fs::create_dir_all(paths.config_dir())?;
+
+        Self::init_data_file(paths.medications_path(), DEFAULT_MEDICATIONS)?;
+        Self::init_data_file(paths.dose_records_path(), DEFAULT_DOSE_RECORDS)?;


initialize() only calls create_dir_all() for paths.config_dir(), but the data file paths can be overridden via env vars to locations outside that directory. If an override points to a file in a non-existent parent directory, fs::write will fail. Consider creating parent directories for each resolved file path (and erroring early if a path exists but is not a regular file).

Copilot · 2026-03-08T02:34:54Z

src/infrastructure/config/app_initializer.rs

+        if let (Some(user_map), Value::Object(default_map)) =
+            (existing.as_object_mut(), defaults)
+        {


merge_default_settings() silently does nothing when settings.json exists but is not a JSON object (existing.as_object_mut() returns None), leaving the app without required default keys. Consider returning an InvalidData error (or rewriting to defaults) when the existing settings are not an object so corruption/misformat is handled explicitly.

Suggested change

if let (Some(user_map), Value::Object(default_map)) =

(existing.as_object_mut(), defaults)

{

let user_map = match existing.as_object_mut() {

Some(map) => map,

None => {

return Err(io::Error::new(

io::ErrorKind::InvalidData,

"settings.json must contain a JSON object",

));

}

};

if let Value::Object(default_map) = defaults {

gbrennon added 4 commits March 7, 2026 23:26

chore(deps): add dirs crate for XDG config path resolution

e42d280

feat(infrastructure): resolve data files from ~/.config/bitpill/

735913e

test(infrastructure): wire harness and fix stale dormant tests

5366f66

test(infrastructure): add AppInitializer integration tests

d8c997b

Copilot AI review requested due to automatic review settings March 8, 2026 02:28

Copilot started reviewing on behalf of gbrennon March 8, 2026 02:29 View session

gbrennon self-assigned this Mar 8, 2026

gbrennon merged commit 6733c1c into main Mar 8, 2026
2 checks passed

gbrennon deleted the feat/xdg-config-dir branch March 8, 2026 02:29

Copilot AI reviewed Mar 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/xdg config dir#31

feat/xdg config dir#31
gbrennon merged 4 commits intomainfrom
feat/xdg-config-dir

gbrennon commented Mar 8, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Copilot AI Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        if let (Some(user_map), Value::Object(default_map)) =
-            (existing.as_object_mut(), defaults)
-        {
+        let user_map = match existing.as_object_mut() {
+            Some(map) => map,
+            None => {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidData,
+                    "settings.json must contain a JSON object",
+                ));
+            }
+        };
+        if let Value::Object(default_map) = defaults {

Conversation

gbrennon commented Mar 8, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants