Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Create a cmdline interface for testing Context creation + Complete function #5256

Merged
merged 10 commits into from
Dec 14, 2021
Merged

Create a cmdline interface for testing Context creation + Complete function #5256

merged 10 commits into from
Dec 14, 2021

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Nov 29, 2021 •
edited
Loading

What type of PR is this?

/kind cleanup

What does this PR do / why we need it:

This PR:

  • creates an abstraction for accessing the Cobra library from the genericclioptions.New() function and from the Complete() functions.
  • moves some client-go code from the Context package to the kclient one.
  • removes the Cobra dependency (parameter) to the Run() function, where is was (nearly) never used
  • adds a unit tests for genericclioptions.New() function

This changes make possible to test the creation of the context and the Complete function without dependencies (except filesystem dependency, because I cannot find a way to use the devfile library on a fake filesystem).

Which issue(s) this PR fixes:

Fixes #?

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

@feloy feloy changed the title Create a cmdline interface for testing Complete/Valdate functions [wip] Create a cmdline interface for testing Complete/Valdate functions Nov 29, 2021
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Nov 29, 2021
@openshift-ci openshift-ci bot requested review from anandrkskd and dharmit November 29, 2021 07:12
@feloy feloy force-pushed the refactor/cmdline-interface branch from db1132d to 66f275d Compare November 29, 2021 07:27
@feloy feloy changed the title [wip] Create a cmdline interface for testing Complete/Valdate functions [wip] Create a cmdline interface for testing Complete/Validate functions Nov 29, 2021
@feloy feloy force-pushed the refactor/cmdline-interface branch 6 times, most recently from aaf77a9 to 094f910 Compare November 29, 2021 19:43
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
13.5% 13.5% Duplication

@feloy feloy changed the title [wip] Create a cmdline interface for testing Complete/Validate functions [wip] Create a cmdline interface for testing Context creation + Complete function Nov 30, 2021
@feloy feloy force-pushed the refactor/cmdline-interface branch from 094f910 to 15ed82a Compare December 2, 2021 14:21
@netlify
Copy link

netlify bot commented Dec 2, 2021 •
edited
Loading

✔️ Deploy Preview for odo-docusaurus-preview canceled.

🔨 Explore the source changes: d3ef6af

🔍 Inspect the deploy log: https://app.netlify.com/sites/odo-docusaurus-preview/deploys/61b787536d617e00088a1886

@feloy feloy force-pushed the refactor/cmdline-interface branch from 15ed82a to a793b93 Compare December 2, 2021 14:24
@feloy feloy changed the title [wip] Create a cmdline interface for testing Context creation + Complete function Create a cmdline interface for testing Context creation + Complete function Dec 2, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Dec 2, 2021
@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

Unit Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

/test psi-kubernetes-integration-e2e

Same problem as before the fix by Anand on Redis ClusterRole

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

Kubernetes Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

Unit Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

OpenShift Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

Kubernetes Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

OpenShift Tests finished.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 2, 2021

/test psi-kubernetes-integration-e2e

[Fail] odo link command tests for OperatorHub Operators are installed in the cluster when a component and a service are deployed when a link with between the component and the service is created with --bind-as-files and --inlined when odo push is executed [BeforeEach] should display the link in odo describe

@feloy feloy force-pushed the refactor/cmdline-interface branch from a793b93 to 971bcc0 Compare December 3, 2021 07:33
@feloy
Copy link
Contributor Author

feloy commented Dec 3, 2021

/test psi-kubernetes-integration-e2e

[Fail] odo link command tests for OperatorHub Operators are installed in the cluster when a component and a service are deployed when a link between the component and the service is created when odo push is executed [BeforeEach] should not list the service binding in `odo service list`

@feloy feloy force-pushed the refactor/cmdline-interface branch from 10366c9 to d3ef6af Compare December 10, 2021 12:56
@feloy
Copy link
Contributor Author

feloy commented Dec 10, 2021

Kubernetes Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 10, 2021

OpenShift Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 10, 2021

Unit Tests finished successfully.
View logs: TXT HTML

@feloy feloy requested a review from valaparthvi December 13, 2021 07:46
@feloy feloy force-pushed the refactor/cmdline-interface branch from 1483f4f to d3ef6af Compare December 13, 2021 17:48
@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

Kubernetes Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

Unit Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

OpenShift Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

Kubernetes Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

Unit Tests finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Dec 13, 2021

OpenShift Tests finished successfully.
View logs: TXT HTML

@valaparthvi
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Dec 14, 2021
@valaparthvi
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Dec 14, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: valaparthvi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Dec 14, 2021
@feloy
Copy link
Contributor Author

feloy commented Dec 14, 2021

/test psi-kubernetes-integration-e2e

Timeout after 4h

1 similar comment
@feloy
Copy link
Contributor Author

feloy commented Dec 14, 2021

/test psi-kubernetes-integration-e2e

Timeout after 4h

@openshift-merge-robot openshift-merge-robot merged commit c58e4c7 into redhat-developer:main Dec 14, 2021
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anandrkskd anandrkskd Awaiting requested review from anandrkskd

@dharmit dharmit Awaiting requested review from dharmit

@valaparthvi valaparthvi Awaiting requested review from valaparthvi

Assignees

@valaparthvi valaparthvi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. area/refactoring Issues or PRs related to code refactoring lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@feloy @kadel @valaparthvi @rm3l @openshift-merge-robot