Skip to content

Fix UX issue when exec command fails to run as part of a deploy command#6673

Merged
openshift-merge-robot merged 1 commit intoredhat-developer:mainfrom
rm3l:fix_ux_issue_when_exec_command_fails_with_odo_deploy
Mar 23, 2023
Merged

Fix UX issue when exec command fails to run as part of a deploy command#6673
openshift-merge-robot merged 1 commit intoredhat-developer:mainfrom
rm3l:fix_ux_issue_when_exec_command_fails_with_odo_deploy

Conversation

@rm3l
Copy link
Member

@rm3l rm3l commented Mar 22, 2023

What type of PR is this:
/kind bug
/area UX
/area deploy

What does this PR do / why we need it:
I stumbled upon this issue while reviewing #6672.

Before the changes, the Execution output: message was displayed on the same line as the spinner, like so:

↪ Executing command:
◓  Executing command in container (command: exec-deploy)Execution output:
/bin/sh: line 1: helm: command not found
 ✗  Executing command in container (command: exec-deploy) [8s]
 ✗  failed to execute (command: exec-deploy)

This is now fixed by ending the spinner as soon as the command is done, and then displaying the command output. Example output:

↪ Executing command:
 ✗  Executing command in container (command: exec-deploy) [8s]
Execution output:
/bin/sh: line 1: helm: command not found
 ✗  failed to execute (command: exec-deploy)

Which issue(s) this PR fixes:
Related to #5701

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@rm3l
Fix UX issue when exec command fails to run as part of a deploy command
aa675ae 
For the context, before the changes, the "Execution output" message
was displayed on the same line as the spinner, e.g.:

```
↪ Executing command:
◓  Executing command in container (command: exec-deploy)Execution output:
/bin/sh: line 1: helm: command not found
 ✗  Executing command in container (command: exec-deploy) [8s]
 ✗  failed to execute (command: exec-deploy)
```

This is now fixed by ending the spinner as soon as possible,
and then displaying the command output:

```
↪ Executing command:
 ✗  Executing command in container (command: exec-deploy) [8s]
Execution output:
/bin/sh: line 1: helm: command not found
 ✗  failed to execute (command: exec-deploy)

```
@rm3l rm3l added area/UX Issues or PRs related to User Experience area/deploy Issues or PRs specific to the `odo deploy` command labels Mar 22, 2023
@netlify
Copy link

netlify bot commented Mar 22, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit aa675ae
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/641b2aaa2a01710008fdc6a6

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 22, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and feloy March 22, 2023 16:20
@rm3l rm3l requested review from valaparthvi and removed request for anandrkskd March 22, 2023 16:20
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

OpenShift Unauthenticated Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

NoCluster Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

Unit Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

Validate Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

Kubernetes Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

Windows Tests (OCP) on commit finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

OpenShift Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Mar 22, 2023

Kubernetes Docs Tests on commit 47234b9 finished successfully.
View logs: TXT HTML

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 23, 2023
@rm3l rm3l added this to the v3.9.0 🚀 milestone Mar 23, 2023
@rm3l
Copy link
Member Author

rm3l commented Mar 23, 2023 

  [FAILED] Expected
      <*url.Error | 0xc000439260>: {
          Op: "Post",
          URL: "http://127.0.0.1:51302/api/newuser",
          Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3457/tests/e2escenarios/e2e_test.go:306 @ 03/22/23 11:44:12.853

  There were additional failures detected.  To view them in detail run ginkgo -vv
------------------------------

Summarizing 1 Failure:
  [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
  C:/Users/Administrator.ANSIBLE-TEST-VS/3457/tests/e2escenarios/e2e_test.go:306

Flaky E2E test - reported in #6582

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented Mar 23, 2023

@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test

Details

In response to this:

 [FAILED] Expected
     <*url.Error | 0xc000439260>: {
         Op: "Post",
         URL: "http://127.0.0.1:51302/api/newuser",
         Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3457/tests/e2escenarios/e2e_test.go:306 @ 03/22/23 11:44:12.853

 There were additional failures detected.  To view them in detail run ginkgo -vv
------------------------------

Summarizing 1 Failure:
 [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
 C:/Users/Administrator.ANSIBLE-TEST-VS/3457/tests/e2escenarios/e2e_test.go:306

Flaky E2E test - reported in #6582

/override windows-integration-test/Windows-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-merge-robot openshift-merge-robot merged commit 2ef5316 into redhat-developer:main Mar 23, 2023
@rm3l rm3l deleted the fix_ux_issue_when_exec_command_fails_with_odo_deploy branch March 23, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@valaparthvi valaparthvi Awaiting requested review from valaparthvi

1 more reviewer

@feloy feloy feloy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@feloy feloy

Labels

area/deploy Issues or PRs specific to the `odo deploy` command area/UX Issues or PRs related to User Experience kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. Required by Prow.

Projects

None yet

Milestone

v3.9.0 🚀

Development

Successfully merging this pull request may close these issues.

3 participants

@rm3l @feloy @openshift-merge-robot