Open
Conversation
use trap to cleanup the drconfig.yaml file; create separate info/error/success convenience logging fns
use LASTEXITCODE for a stronger validation of dr auth check; try-finally to cleanup drconfig.yaml and powershell completion; stronger validation of dr help; simplified validation of dr self version;
ajalon1
commented
Feb 13, 2026
Contributor
Author
There was a problem hiding this comment.
Most of the changes here are whitespace from the try-finally. Don't know a better way to do this like trap cleanup in bash.
Comment on lines
-92
to
-97
| Write-Delimiter "Execute dr help run" | ||
| dr help run | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-ErrorMsg "dr help run command failed" | ||
| } | ||
| Write-End |
Contributor
Author
There was a problem hiding this comment.
LOL this could be more useful.
Comment on lines
-106
to
-121
| # Test completion generation | ||
| Write-Delimiter "Testing completion generation" | ||
| $completion_file = "completion_powershell.ps1" | ||
| dr self completion powershell > $completion_file | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-ErrorMsg "dr self completion powershell command failed" | ||
| } | ||
| Write-Delimiter "Execute dr self version" | ||
| $version_json = dr self version --format=json | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-ErrorMsg "dr self version command failed" | ||
| } | ||
|
|
||
| # Check if completion file contains expected content | ||
| if (Test-Path $completion_file) { | ||
| $completion_content = Get-Content $completion_file -Raw | ||
| if ($completion_content -match "Register-ArgumentCompleter") { | ||
| Write-SuccessMsg "Assertion passed: We have expected $completion_file file with Register-ArgumentCompleter." | ||
| Remove-Item $completion_file -Force | ||
| # Simplified version key check for PowerShell | ||
| if ($version_json -match '"version":') { | ||
| Write-SuccessMsg "Version command returned expected 'version' key in json output." | ||
| } else { | ||
| Write-ErrorMsg "Assertion failed: We don't have expected $completion_file file with expected function." |
Contributor
Author
There was a problem hiding this comment.
This just got moved down a bit
Comment on lines
+195
to
203
| } finally { | ||
| Write-InfoMsg "Cleaning up..." | ||
| if (Test-Path $testing_dr_cli_config_dir) { | ||
| Remove-Item -Recurse -Force $testing_dr_cli_config_dir | ||
| } | ||
| if (Test-Path "completion_powershell.ps1") { | ||
| Remove-Item -Force "completion_powershell.ps1" | ||
| } | ||
| } |
Comment on lines
+5
to
+18
| # Helper functions | ||
| error() { | ||
| echo "❌ $1" | ||
| exit 1 | ||
| } | ||
|
|
||
| success() { | ||
| echo "✅ $1" | ||
| } | ||
|
|
||
| info() { | ||
| echo "ℹ️ $1" | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
I could be persuaded to use different names for these lol
Contributor
|
🔐 Smoke tests approved by maintainer ⏳ Running security scans before executing smoke tests with secrets... A maintainer has approved this fork PR to run smoke tests. Security scans will run first. |
Contributor
|
❌ Some smoke tests failed. (Fork PR) ✅ Security Scan: success |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RATIONALE
Just doing some minor tweaks to the CLI smoke tests since I'm in this part of the repo anyway.
CHANGES
TODO
Cleanup changes were not necessary!
On the Ubuntu and MacOS side
Windows side
dr help, rather than check the exit codedr self versionPR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation testsLabels: Apply labels to trigger workflows:
run-smoke-testsorgo- Run smoke tests on demand (only works for non-forked PRs)Important
For Forked PRs: If you're an external contributor, the
run-smoke-testslabel won't work. Only maintainers can trigger smoke tests on forked PRs by applying theapproved-for-smoke-testslabel after security review. Please comment requesting maintainer review if you need smoke tests to run.