-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix prompt text test occasional failure #7722
Conversation
Fixes issue radius-project#7721 Signed-off-by: Brooke Hamilton <brooke@azuredev.io>
Signed-off-by: Brooke Hamilton <brooke@azuredev.io>
Thanks @brooke-hamilton for digging in and fixing this. Taking a look.... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging into this!
...ting/contributing-code/contributing-code-first-commit/first-commit-04-debugging-cli/index.md
Show resolved
Hide resolved
Signed-off-by: Brooke Hamilton <brooke@azuredev.io>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7722 +/- ##
==========================================
- Coverage 61.00% 60.99% -0.02%
==========================================
Files 520 520
Lines 27010 27010
==========================================
- Hits 16478 16474 -4
- Misses 9080 9082 +2
- Partials 1452 1454 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great, and a big improvement to the readability of the tests.
@ytimocin - can you help get this merged? |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
# Description Fixes an issue in which the `confirm value` test in `/pkg/cli/prompt/text` sometimes fails due to the timing of the text rendering in the test model. The test was checking the output before calling `TestModel.FinalModel()`, which resulted in an empty result most of the time, but not all of the time. The other assertions made after calling `FinalModel()` showed that the code was operating as designed, which means we can remove the interim check that tests for an empty output. This change simply removes the code that checks the output before calling `FinalModel()`. ## Type of change This pull request fixes a bug in Radius and has an approved issue. Fixes: #7721 --------- Signed-off-by: Brooke Hamilton <brooke@azuredev.io> Co-authored-by: Brooke Hamilton <brooke@azuredev.io> Co-authored-by: Ryan Nowak <nowakra@gmail.com>
# Description Fixes an issue in which the `confirm value` test in `/pkg/cli/prompt/text` sometimes fails due to the timing of the text rendering in the test model. The test was checking the output before calling `TestModel.FinalModel()`, which resulted in an empty result most of the time, but not all of the time. The other assertions made after calling `FinalModel()` showed that the code was operating as designed, which means we can remove the interim check that tests for an empty output. This change simply removes the code that checks the output before calling `FinalModel()`. ## Type of change This pull request fixes a bug in Radius and has an approved issue. Fixes: radius-project#7721 --------- Signed-off-by: Brooke Hamilton <brooke@azuredev.io> Co-authored-by: Brooke Hamilton <brooke@azuredev.io> Co-authored-by: Ryan Nowak <nowakra@gmail.com> Signed-off-by: Reshma Abdul Rahim <reshmarahim.abdul@microsoft.com>
Description
Fixes an issue in which the
confirm value
test in/pkg/cli/prompt/text
sometimes fails due to the timing of the text rendering in the test model.The test was checking the output before calling
TestModel.FinalModel()
, which resulted in an empty result most of the time, but not all of the time. The other assertions made after callingFinalModel()
showed that the code was operating as designed, which means we can remove the interim check that tests for an empty output. This change simply removes the code that checks the output before callingFinalModel()
.Type of change
This pull request fixes a bug in Radius and has an approved issue.
Fixes: #7721