Fixes #3394 Add registration for OSPSuite.Core.ICoreUserSettings in P…

[rwmcintosh]

Fixes #3394

Description

Adding registration for OSPSuite.Core.ICoreUserSettings as required

Type of change

Please mark relevant options with an x in the brackets.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires documentation changes (link at least one user or developer documentation issue):
• Algorithm update - updates algorithm documentation/questions/answers etc.
• Other (please describe):

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Integration tests
• Unit tests
• Manual tests
• No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

• Check if the code is well documented
• Check if the behavior is what is expected
• Check if the code is well tested
• Check if the code is readable and well formatted
• Additional checks (document below if any)
• Check if documentation update issue(s) are created if the option This change requires a documentation update above is selected

Screenshots (if appropriate):

Questions (if appropriate):

Summary by CodeRabbit

• Tests

  • Enhanced integration test infrastructure for R with new test context, helpers, and specs validating snapshot export and cleanup.

• Chores

  • Added sample simulation data and a Data project folder for test fixtures.

• Refactor

  • Adjusted startup service registration to align internal wiring (no user-facing behavior change).

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaces a DI registration to expose OSPSuite.Core.ICoreUserSettings (with ICoreUserSettings alias) and adds R integration test infrastructure, helpers, test data, and a RunSnapshot integration test that validates output .pksim5.

Changes

Cohort / File(s)	Summary
Dependency Injection Configuration src/PKSim.R/Bootstrap/ApplicationStartup.cs	Changed service registration from container.Register<ICoreUserSettings, CLIUserSettings>(LifeStyle.Singleton) to container.Register<OSPSuite.Core.ICoreUserSettings, ICoreUserSettings, CLIUserSettings>(LifeStyle.Singleton) (exposes OSPSuite.Core interface).
Test Infrastructure tests/PKSim.R.Tests/ContextForStaticIntegration.cs, tests/PKSim.R.Tests/DomainHelperForSpecs.cs, tests/PKSim.R.Tests/PKSim.R.Tests.csproj	Added ContextForStaticIntegration base class for integration tests, a DomainHelperForSpecs.DataFilePathFor helper, and included a Data folder in the test project.
RunSnapshot API Test & Data tests/PKSim.R.Tests/RunSnapshotSpecs.cs, tests/PKSim.R.Tests/Data/Atazanavir-Model_1Sim.json	Added RunSnapshotSpecs integration test that prepares input, invokes Api.RunSnapshot, asserts output contains Atazanavir-Model_1Sim.pksim5, and added the JSON model used by the test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Open-Systems-Pharmacology/PK-Sim#3389 — Modifies CLIUserSettings/user-settings classes that interact with the same DI registration and implementation changes in this PR.

Poem

🐰 I hopped through folders, files in tow,
I nudged the settings so the DI would know,
I dropped a model and a test to run,
Snapshots saved when R meets the sun,
A tiny rabbit claps — the checks are done!

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	The PR introduces test infrastructure changes (ContextForStaticIntegration, DomainHelperForSpecs, RunSnapshotSpecs, test data file) that support integration testing but extend beyond the narrow scope of registering the interface.	Clarify whether the test infrastructure additions are necessary for validating the registration change or represent scope creep. Consider whether they should be in a separate PR.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding registration for OSPSuite.Core.ICoreUserSettings, which is demonstrated by the service registration update in ApplicationStartup.cs.
Linked Issues check	✅ Passed	The PR fulfills the linked issue requirement by registering OSPSuite.Core.ICoreUserSettings in ApplicationStartup.cs and adds integration tests validating the API functionality.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 3394-add-registration-for-ospsuitecoreicoreusersettings-in-pksimr

---

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/PKSim.R.Tests/RunSnapshotSpecs.cs`:
- Line 36: The test currently calls File.Exists(Path.Join(_outputFolder,
"Atazanavir-Model_1Sim.pksim5")) but discards the bool result so the assertion
never runs; change this to assert the existence using OSPSuite.BDDHelper (e.g.
call ShouldBeTrue() on the File.Exists(...) result or compare it with true using
ShouldBeEqualTo()), referencing File.Exists, Path.Join, _outputFolder and the
file name "Atazanavir-Model_1Sim.pksim5" so the test fails when the file is not
created.

🧹 Nitpick comments (2)

tests/PKSim.R.Tests/DomainHelperForSpecs.cs (1)

8-9: Misleading comment: the Data folder belongs to PKSim.R.Tests, not PKSim.Tests.

The comment says "Borrowing test data from PKSim.Tests" but the Data\ directory was added to this project (PKSim.R.Tests). Consider updating the comment to avoid confusion.

tests/PKSim.R.Tests/RunSnapshotSpecs.cs (1)

13-14: Hardcoded C:\ root paths are fragile and may require elevated permissions.

Writing directly to C:\Input\ and C:\Output\ can fail on locked-down CI agents or developer machines without admin rights, and risks collisions with other processes. Consider using Path.GetTempPath() or a similar per-run temporary directory.

♻️ Suggested approach

-   private readonly string _inputFolder = @"C:\Input\";
-   private readonly string _outputFolder = @"C:\Output\";
+   private readonly string _inputFolder = Path.Combine(Path.GetTempPath(), "PKSimRTests", "Input");
+   private readonly string _outputFolder = Path.Combine(Path.GetTempPath(), "PKSimRTests", "Output");