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.

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

Add test-case for validating devfiles on component create #3908

Merged

Conversation

johnmcollier
Copy link
Member

@johnmcollier johnmcollier commented Sep 8, 2020

What type of PR is this?
/kind bug

What does does this PR do / why we need it:
Currently, a user can pass in any file to odo create, and it will always succeed provided the file exists, either on disk or as part of a registry, nothing tells them if their devfile is valid or not. No validation is performed until the user tries to push the component.

This PR updates the odo create functionality to properly validate devfiles when a component is created. If an invalid devfile is specified, a validation error will be thrown. I've also left the validation in for odo push, as we should still be validating on each push as well.

Adds an integration test case to validate that odo create validates the devfile

Which issue(s) this PR fixes:

Fixes #3778

PR acceptance criteria:

How to test changes / Special notes to the reviewer:
Run the odo create integration tests

Signed-off-by: John Collier <jcollier@redhat.com>
Signed-off-by: John Collier <jcollier@redhat.com>
Signed-off-by: John Collier <jcollier@redhat.com>
@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 8, 2020
Signed-off-by: John Collier <jcollier@redhat.com>
…eate

Signed-off-by: John Collier <jcollier@redhat.com>
@johnmcollier
Copy link
Member Author

johnmcollier commented Sep 9, 2020

Rebased the PR. Since #3867 included the fix also in this PR, this PR has been updated to just include the test case for the scenario

Signed-off-by: John Collier <jcollier@redhat.com>
@johnmcollier johnmcollier changed the title Validate devfiles on component create [Test] Validate devfiles on component create Sep 9, 2020
@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #3908 into master will increase coverage by 0.17%.
The diff coverage is 70.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3908      +/-   ##
==========================================
+ Coverage   44.60%   44.77%   +0.17%     
==========================================
  Files         143      143              
  Lines       13860    13923      +63     
==========================================
+ Hits         6182     6234      +52     
- Misses       7089     7097       +8     
- Partials      589      592       +3     
Impacted Files Coverage Δ
pkg/component/component.go 25.76% <0.00%> (ø)
pkg/devfile/parser/representation.go 0.00% <0.00%> (ø)
pkg/envinfo/envinfo.go 43.93% <0.00%> (-0.52%) ⬇️
...g/devfile/adapters/kubernetes/component/adapter.go 32.93% <9.09%> (-0.41%) ⬇️
pkg/devfile/adapters/kubernetes/utils/utils.go 58.71% <58.82%> (+3.57%) ⬆️
pkg/url/url.go 66.85% <81.57%> (+1.01%) ⬆️
pkg/devfile/parser/configurables.go 57.25% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7831c92...b263790. Read the comment docs.

@johnmcollier johnmcollier changed the title [Test] Validate devfiles on component create Add test-case for validating devfiles on component create Sep 14, 2020
@johnmcollier
Copy link
Member Author

@mik-dass @prietyc123 Can I get a review on this? Thanks!

@girishramnani
Copy link
Contributor

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 16, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Sep 16, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 14d656b into redhat-developer:master Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. 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
Development

Successfully merging this pull request may close these issues.

'odo create' should properly validate devfiles
5 participants