search

fabric8-ui / fabric8-planner

https://fabric8-ui.github.io/fabric8-planner/
Apache License 2.0
26 stars 62 forks source link

fix(dynamic-component): Remove None from Enum field values #2798

Closed jarifibrahim closed 5 years ago

jarifibrahim commented 5 years ago

Fixes https://github.com/openshiftio/openshift.io/issues/3831 and https://github.com/openshiftio/openshift.io/issues/3832#issuecomment-433874194

This PR removes the unnecessary None field in the dropdown for enum types. The backend would always send a default value for enum fields.

Before

Now

screenshot from 2018-11-01 17-26-20

alien-ike commented 5 years ago

Ike Plugins (test-keeper)

Thank you @jarifibrahim for this contribution!

It appears that no tests have been added or updated in this PR.

Automated tests give us confidence in shipping reliable software. Please add some as part of this change.

If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command /ok-without-tests as a comment to make the status green.

Your plugin configuration is stored in the file.

centos-ci commented 5 years ago

@jarifibrahim Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 and visit http://localhost:5000 to access it.

jarifibrahim commented 5 years ago

[test]

centos-ci commented 5 years ago

@jarifibrahim Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2798 and visit http://localhost:5000 to access it.

sahil143 commented 5 years ago

/ok-without-tests

kwk commented 5 years ago

/ok-without-tests

@sahil143 how can this be okay without a test?

jarifibrahim commented 5 years ago

/ok-without-tests

@sahil143 how can this be okay without a test?

@kwk We don't have the tests for the dropdowns. We test user interactions via e2e tests and we test services via unit test. The piece of code I modified doesn't have any test (and it's not not easy to test it because it changes some HTML. You can't easily test rendered HTML)

kwk commented 5 years ago

The piece of code I modified doesn't have any test (and it's not not easy to test it because it changes some HTML. You can't easily test rendered HTML

I guess we have to find a way to test it though. Relying on tests that run in a different repo is not very cool because you cannot verify this. If everything was in monorepo things looks different I guess.

jarifibrahim commented 5 years ago

ind a way to test it though. Relying on tests that run in a different repo is not very cool because you cannot verify this. If everything was in monorepo things looks different I guess.

@kwk No no. I can assure you this PR doesn't break anything :wink: . We have our own e2e tests. The PR build executed the unit tests and about 68 tests functional Tests (aka e2e tests) on each SDD and AGILE template. We cover most of the user interaction scenario via these e2e-tests.

The QE team has their own e2e tests which cover the entire osio workflow (build, launcher, etc) and we have our own e2e tests that cover common planner workflows (only planner).