[test] fix start_test failing to import SECOM_CONFIG from stream_test

[pieleric]

Commit da7ef8e (Extend SPARC acquisition to support rotation)
updated the stream_test and broke it into different files. However, this
broken start_test which relied on it to define SECOM_CONFIG.

=> Just hard-code separately SECOM_CONFIG in start_test.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies src/odemis/odemisd/test/start_test.py by adding module-level constants for file path management. Two new constants are introduced: CONFIG_PATH, which computes the installation directory path, and SECOM_CONFIG, which builds the full path to a SECOM configuration file. The import of SECOM_CONFIG from odemis.acq.test.stream_test is replaced with local imports of os and odemis, shifting the source of this configuration path from a dependency module to this test module itself.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing start_test's import of SECOM_CONFIG from stream_test by redefining it locally.
Description check	✅ Passed	The description is directly related to the changeset, explaining the root cause (commit da7ef8e broke stream_test) and the solution (hard-coding SECOM_CONFIG in start_test).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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: 0

🧹 Nitpick comments (2)

src/odemis/odemisd/test/start_test.py (2)

14-14: Use os.path.join() and os.path.normpath() for path construction.

The current implementation uses string concatenation for path construction, which is error-prone. Replace with os.path.join() for clarity and os.path.normpath() to properly resolve the ../../ navigation.

♻️ Proposed refactor

-CONFIG_PATH = os.path.dirname(odemis.__file__) + "/../../install/linux/usr/share/odemis/"
+CONFIG_PATH = os.path.normpath(os.path.join(os.path.dirname(odemis.__file__), "..", "..", "install", "linux", "usr", "share", "odemis"))

---

15-15: Consider using os.path.join() for cross-platform path construction.

Both line 14 and line 15 use string concatenation for path construction. While the current approach works (the config file exists at the constructed path), using os.path.join() is better practice for portability across different operating systems:

CONFIG_PATH = os.path.join(os.path.dirname(odemis.__file__), "..", "..", "install", "linux", "usr", "share", "odemis")
SECOM_CONFIG = os.path.join(CONFIG_PATH, "sim", "secom-sim.odm.yaml")

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e44fd47 and 845aa66.

📒 Files selected for processing (1)

• src/odemis/odemisd/test/start_test.py

🧰 Additional context used

🧬 Code graph analysis (1)

src/odemis/odemisd/test/start_test.py (1)

src/odemis/odemisd/start.py (1)

• find_window (600-623)

🪛 GitHub Actions: Linting

src/odemis/odemisd/test/start_test.py

[error] 1-1: Contains forbidden metadata chunks (pngcheck detected prohibited PNG metadata).

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: CodeQL analysis (python)
• GitHub Check: build (ubuntu-24.04)
• GitHub Check: build (ubuntu-22.04)

🔇 Additional comments (1)

src/odemis/odemisd/test/start_test.py (1)

1-1: LGTM: Import additions are appropriate.

The additions of os and odemis imports are necessary and correct for constructing the configuration path locally.

Also applies to: 10-10

[Copilot]

Pull request overview

This PR fixes a broken import in start_test.py that was caused by refactoring of stream_test module. The test file previously imported SECOM_CONFIG from odemis.acq.test.stream_test, but after the module was split into multiple files, this import no longer works. The solution hard-codes the SECOM_CONFIG path definition directly in start_test.py instead.

Key changes:

• Replaced broken import with local definition of SECOM_CONFIG
• Added os and odemis imports to support the path construction
• Defined CONFIG_PATH and SECOM_CONFIG constants using the same pattern as other test files in the codebase

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.