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

Gherkin form based edit build config #9840

Conversation

sanketpathak
Copy link
Contributor

@sanketpathak sanketpathak commented Aug 19, 2021

Epic: https://issues.redhat.com/browse/ODC-5008
Story: https://issues.redhat.com/browse/ODC-5968

Acceptance criteria:

  • The Edit Build Config action should allow the user to have a form driven experience
  • The Edit Build Config form driven experience should support the same features that were supported in 3.x

Checks required for approving Epic gherkin scripts PR:

  • Add @epic-number to the scenarios or feature file [e.g: @odc-xxx]
  • Add @to-do for automation possible scenarios
  • Add @regression or @smoke based on the importance of the scenario
  • Update the test scenarios count in Automation status confluence Report
  • Check for the linter issues by executing yarn run gherkin-lint on frontend folder [Skip epic number tags related linter issues]

@openshift-ci openshift-ci bot added the component/dev-console Related to dev-console label Aug 19, 2021
@sanketpathak sanketpathak force-pushed the gherkin-form-based-edit-build-config branch 2 times, most recently from 5d0183e to 69b766f Compare August 19, 2021 15:37
@sanketpathak
Copy link
Contributor Author

/assign @divyanshiGupta @gajanan-more

@sanketpathak
Copy link
Contributor Author

/cc @jerolimov

@openshift-ci openshift-ci bot requested a review from jerolimov August 19, 2021 15:42
@sanketpathak sanketpathak force-pushed the gherkin-form-based-edit-build-config branch 3 times, most recently from 6c85b01 to a5c38ba Compare August 20, 2021 10:28
Copy link
Contributor

@gajanan-more gajanan-more left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 20, 2021
Copy link
Member

@jerolimov jerolimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far, added some small comments about cleanup yaml files and some alignments with the current implementation in WIP PR #9834

When user navigates to build tab
And user clicks on kebab menu for "nodejs-ex-git1" build config
And user clicks on Edit BuildConfig
Then user will see the Git Repository url, Image configuration, Environment Variables
Copy link
Member

@jerolimov jerolimov Aug 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name field should also be also shown:

Suggested change
Then user will see the Git Repository url, Image configuration, Environment Variables
Then user will see the Name, Git repository url, Image configuration, Environment Variables

Btw. Isn't it easier later to implement this when you check things seperate?

Suggested change
Then user will see the Git Repository url, Image configuration, Environment Variables
Then user will see the "Name" field
And user will see the "Git repo URL" field
And user will see the "Images" section
And user will see the "Environment Variables" section

This is really just a question as I don't implemented these page objects and actions later.

Comment on lines 22 to 23
When user clicks on Show advanced options
Then user will see the Triggers, Build Secret, Run Policy, Post-Commit Hooks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user needs multiple clicks because that are multiple advanced options similar to the add page.

Maybe like this: ... Or you create multiple tests for each case?

Suggested change
When user clicks on Show advanced options
Then user will see the Triggers, Build Secret, Run Policy, Post-Commit Hooks
When user clicks on Advanced option "Triggers"
And user clicks on Advanced option "Secrets"
And user clicks on Advanced option "Run Policy"
And user clicks on Advanced option "Hooks"
Then user will see section "Triggers"
And user will see section "Build Secret"
And user will see section "Run Policy"
And user will see section "Hooks"

Scenario: Edit Advanced git options of buildconfig which uses strategy Docker and Git as source: EBC-01-TC05
# user can use buildconfig-with-strategy-docker-source-git.yaml from testData/yamls/BuildConfig
Given user has applied the yaml "buildconfig-with-strategy-docker-source-git.yaml"
And user is at Edit Build Config page
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
And user is at Edit Build Config page
And user is at Edit Build Config page of deployment "flask-app-12"

Comment on lines +62 to +75
# user can use buildconfig-with-strategy-docker-source-dockerfile.yaml from testData/yamls/BuildConfig
Given user has applied the yaml "buildconfig-with-strategy-docker-source-dockerfile.yaml"
When user clicks on action menu of build config
And user selects the option Edit BuildConfig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above you used an sentence which brings you directly to this page. You should/could use it here as well or?

Suggested change
# user can use buildconfig-with-strategy-docker-source-dockerfile.yaml from testData/yamls/BuildConfig
Given user has applied the yaml "buildconfig-with-strategy-docker-source-dockerfile.yaml"
When user clicks on action menu of build config
And user selects the option Edit BuildConfig
# user can use buildconfig-with-strategy-docker-source-dockerfile.yaml from testData/yamls/BuildConfig
Given user has applied the yaml "buildconfig-with-strategy-docker-source-dockerfile.yaml"
And user is at Edit Build Config page of deployment "flask-app-12"

Comment on lines 68 to 69
And user goes to Environment tab
Then user will see Name as "<name>" and and Value as "<value>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no environment tab.

Suggested change
And user goes to Environment tab
Then user will see Name as "<name>" and and Value as "<value>"
Then user will see Name as "<name>" and and Value as "<value>" in Environment Variables

Comment on lines 8 to 9
creationTimestamp: '2021-05-04T07:39:42Z'
generation: 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this lines, they will be generated by kubernetes

Suggested change
creationTimestamp: '2021-05-04T07:39:42Z'
generation: 2

Comment on lines 17 to 19
resourceVersion: '1303043788'
selfLink: /apis/build.openshift.io/v1/namespaces/aut-form-edit-build-config/buildconfigs/flask-app-12
uid: e3927409-acab-11eb-8ed8-42010a8e0002
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this lines, they will be generated by kubernetes

Suggested change
resourceVersion: '1303043788'
selfLink: /apis/build.openshift.io/v1/namespaces/aut-form-edit-build-config/buildconfigs/flask-app-12
uid: e3927409-acab-11eb-8ed8-42010a8e0002

Comment on lines 10 to 12
uid: cd73cce1-75b4-484e-9b35-f6c9dc3ed8b6
creationTimestamp: '2021-05-04T07:45:14Z'
generation: 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this lines, they will be generated by kubernetes

Suggested change
uid: cd73cce1-75b4-484e-9b35-f6c9dc3ed8b6
creationTimestamp: '2021-05-04T07:45:14Z'
generation: 2

openshift.io/build-config.name: fruits-app-1
openshift.io/build.number: '1'
openshift.io/build.pod-name: fruits-app-1-1-build
resourceVersion: '338752'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this lines, they will be generated by kubernetes

Suggested change
resourceVersion: '338752'

- apiVersion: build.openshift.io/v1
kind: BuildConfig
name: fruits-app-1
uid: 44291715-6933-4fac-82f1-481ea0fe7f29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this lines, they will be generated by kubernetes

Suggested change
uid: 44291715-6933-4fac-82f1-481ea0fe7f29

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried YAML on cluster it threw an error saying this uid is reuired so I've not removed this one

@sanketpathak sanketpathak force-pushed the gherkin-form-based-edit-build-config branch from a5c38ba to 8580cf1 Compare August 20, 2021 17:51
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 20, 2021
@jerolimov
Copy link
Member

Thanks for applying my recommended changes and checking the yaml files. Restore the lgtm from @gajanan-more

/lgtm
/approve

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 21, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gajanan-more, jerolimov, sanketpathak

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 px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR labels Aug 21, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 21, 2021

@jerolimov: The label(s) /label doc-approved cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed

In response to this:

Epic has a doc-ack and px-ack, so approving this on behalf of doc and PX team.

Because this PR only contains disabled (for the moment) gherkin scripts I'm also approving this also for QE.

Cypress skips this new scenarios:

Screenshot from 2021-08-21 13-16-53

/label doc-approved
/label px-approved
/label qe-approved

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.

@jerolimov
Copy link
Member

Epic has a doc-ack and px-ack, so approving this on behalf of doc and PX team.

Because this PR only contains disabled (for the moment) gherkin scripts I'm also approving this also for QE.

Cypress skips this new scenarios:

Screenshot from 2021-08-21 13-16-53

/label docs-approved
/label px-approved
/label qe-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Aug 21, 2021
@openshift-bot
Copy link
Contributor

/retest-required

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-merge-robot openshift-merge-robot merged commit 1db91c3 into openshift:master Aug 21, 2021
@spadgett spadgett added this to the v4.9 milestone Aug 30, 2021
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. component/dev-console Related to dev-console docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants