feat: print onboarding hint after successful install (#1153)#1157
Conversation
After a successful install, both install.sh (Linux/macOS) and install.ps1 (Windows) now always print: Next: run 'opensre onboard' to complete setup. The hint appears unconditionally after configure_path / the PATH warning, so first-time installers and users upgrading an existing install both see it. Adds 5 new tests to tests/cli/test_install_sh_path.py via a new _run_post_install helper that exercises the full post-install output block (version confirmation + configure_path + hint) rather than configure_path in isolation, covering the silent early-return PATH scenario that the existing helper could not reach.
Greptile SummaryThis PR adds a "Next: run Confidence Score: 4/5Safe to merge for end-users, but the new tests won't catch a regression of the feature they claim to test. The installer changes are trivially correct. The P1 finding is in the test file: the helper copies the code under test into the test itself, making all five new assertions permanently green even if the hint is removed from install.sh. This is a real defect in the newly added test infrastructure, so the score is 4 rather than 5. tests/cli/test_install_sh_path.py — _run_post_install helper and all five new tests need to be reworked to actually source the hint from install.sh. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[install.sh / install.ps1 runs] --> B[Download & install binary]
B --> C[Print version confirmation line]
C --> D[configure_path]
D --> E{Install dir already on PATH?}
E -- Yes --> F[Return silently]
E -- No / Windows --> G[Write rc file / emit PATH warning]
F --> H["log: Next: run 'opensre onboard' to complete setup."]
G --> H
Reviews (1): Last reviewed commit: "style: apply ruff formatting to test_ins..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # --- real post-install output block (mirrors install.sh lines 690-703) --- | ||
| if [ "$INSTALL_CHANNEL" = "main" ]; then | ||
| if [ "$installed_version" = "main" ]; then | ||
| log "Installed ${{BIN_NAME}} main build to ${{INSTALL_DIR}}/${{BIN_NAME}}" | ||
| else | ||
| log "Installed ${{BIN_NAME}} main build (${{installed_version}}) to ${{INSTALL_DIR}}/${{BIN_NAME}}" | ||
| fi | ||
| else | ||
| log "Installed ${{BIN_NAME}} v${{installed_version}} to ${{INSTALL_DIR}}/${{BIN_NAME}}" | ||
| fi | ||
|
|
||
| configure_path | ||
|
|
||
| log "" |
There was a problem hiding this comment.
The _run_post_install helper copies lines 690-703 of install.sh verbatim into the test script, including the new log "Next: run 'opensre onboard' to complete setup." line (line 228). Every assertion like assert "opensre onboard" in result.stdout + result.stderr will always pass because the test itself emits that string — it is not read from install.sh. If someone later deletes the hint from install.sh without touching this file, all five new tests continue to pass.
To actually guard against regression, the helper should drive the real top-level block of install.sh (e.g., source the whole script with mocked network/binary helpers and a trapped exit) rather than re-implementing it in Python. Alternatively, a simpler approach is to grep install.sh directly for the hint string as a contract test alongside the subprocess-based scenario tests.
The _run_post_install helper was copying lines 690-703 of install.sh
verbatim into the test script, including the onboarding hint itself.
This made all five new assertions permanently green even if the hint
was removed from install.sh (the test would still emit the string).
Fix: replace the hard-coded block with two changes:
1. _find_post_install_start_line() locates the version-print block
in install.sh at runtime by scanning for its opening line.
2. The helper now uses 'tail -n +N install.sh' fed through eval, so
the subprocess runs the real lines from install.sh directly.
Any deletion or edit of the hint in install.sh will now cause the
five scenario tests to fail as intended.
Also add two contract tests (test_install_sh_contains_onboarding_hint
and test_install_ps1_contains_onboarding_hint) that grep the script
sources directly, providing a fast and independent regression guard
that is completely decoupled from subprocess execution.
|
LGTM 👍 |
|
😤 @Ryjen1 said "I will fix this" and then actually fixed it. Legendary behavior.
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
2 participants
Summary
Fixes #1153.
After a successful install, both
install.sh(Linux/macOS) andinstall.ps1(Windows) now always print a clear next-step message:The hint appears unconditionally after
configure_path/ the PATH warning block, so it shows up in every scenario — first-time install, reinstall, and upgrade (including the silent case where the install dir is already onPATHandconfigure_pathreturns without printing anything).What changed
install.shconfigure_path(blank line + hint)install.ps1Install-OpenSrebefore closing brace (blank line + hint)tests/cli/test_install_sh_path.py_run_post_installhelper + 5 new testsApproach chosen
Option B from the issue — always print a clear next-step message. Option A (auto-running
opensre onboard) was ruled out because the binary may not be onPATHin the current shell session when installed viacurl | bash, making auto-run silently unreliable.Tests
5 new tests added to
tests/cli/test_install_sh_path.py:test_onboarding_hint_shown_when_path_not_setconfigure_pathwrites rc file, hint followstest_onboarding_hint_shown_when_path_already_setconfigure_pathreturns silently, hint still appearstest_onboarding_hint_shown_for_bash_linuxtest_onboarding_hint_shown_for_main_channel--mainrolling build installtest_onboarding_hint_appears_after_version_line"Installed opensre v..."The new
_run_post_installhelper exercises the full real post-install output block ofinstall.sh(version print +configure_path+ hint) rather thanconfigure_pathin isolation, closing the gap where the silent early-return PATH scenario was untestable with the old_run()helper.Before: 12 passed, no assertion on
"opensre onboard"After: 17 passed, 0 failures
Testing locally