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

CONSOLE-2420: Added i18n testing to cypress crud test suite #7213

Merged
merged 1 commit into from Nov 23, 2020
Merged

CONSOLE-2420: Added i18n testing to cypress crud test suite #7213

merged 1 commit into from Nov 23, 2020

Conversation

dtaylor113
Copy link
Contributor

@dtaylor113 dtaylor113 commented Nov 13, 2020 •
edited

Overview

This PR extends our Cypress CRUD tests to execute i18n testing for List and Detail pages for the following resources/kinds:

  • Pod
  • DeploymentConfig
  • Service
  • ConfigMap
  • Ingress
  • CronJob
  • Job
  • DaemonSet
  • Deployment
  • ReplicaSet
  • ReplicationController
  • BuildConfig
  • ImageStream
  • StatefulSet
  • ResourceQuota
  • LimitRange
  • HorizontalPodAutoscaler
  • NetworkPolicy

The following resrouces/kinds have not yet been translated and currently SKIP i18n testing:

  • ClusterServiceBroker
  • Group
  • ServiceAccount
  • Secret
  • PersistentVolume
  • PersistentVolumeClaim
  • StorageClass
  • Role
  • VolumeSnapshot
  • VolumeSnapshotClass
  • VolumeSnapshotContent

Tech Notes

Each List page has I18N testing done on the following elements:

image

Each Details page has I18N testing done on the following elements:

image

i18n-not-translated

I18N tests will skip any elements which have the i18n-not-translated element:

image

Code example:

<dt i18n-not-translated="true" key={`${k}-k`}>
  {k}
</dt>,

@dtaylor113 dtaylor113 self-assigned this Nov 13, 2020
@dtaylor113 dtaylor113 mentioned this pull request Nov 13, 2020
Closed
spadgett
spadgett reviewed Nov 16, 2020
View reviewed changes
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

Thanks @dtaylor113. I think this is cleaner than the other approach.

frontend/packages/integration-tests-cypress/support/i18n.ts Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the component/core Related to console core functionality label Nov 17, 2020
@spadgett spadgett changed the title Cypress: Added i18n testing to cypress crud test suite CONSOLE-2420: Added i18n testing to cypress crud test suite Nov 18, 2020
spadgett
spadgett reviewed Nov 19, 2020
View reviewed changes
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

Thanks @dtaylor113

frontend/packages/integration-tests-cypress/support/i18n.ts Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/support/i18n.ts Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/support/i18n.ts Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/views/list-page.ts Show resolved Hide resolved
frontend/public/components/deployment-config.tsx Outdated Show resolved Hide resolved
frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts Outdated Show resolved Hide resolved
@dtaylor113 dtaylor113 force-pushed the add-i18n-testing-to-crud-tests branch 2 times, most recently from 77ea6d3 to b4d6760 Compare November 19, 2020 20:40
@dtaylor113 dtaylor113 force-pushed the add-i18n-testing-to-crud-tests branch 2 times, most recently from 2adc55b to 697a265 Compare November 20, 2020 18:12
@openshift-ci-robot openshift-ci-robot added the component/olm Related to OLM label Nov 20, 2020
spadgett
spadgett approved these changes Nov 20, 2020
View reviewed changes
Copy link
Member

@spadgett spadgett 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-robot openshift-ci-robot 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 Nov 20, 2020
@spadgett
Copy link
Member

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 20, 2020
@spadgett
Copy link
Member

@dtaylor113 Do you know if this instance needs to be updated?

https://github.com/openshift/console/blob/master/frontend/packages/dev-console/integration-tests/views/pipeline.view.ts#L9

@dtaylor113
Copy link
Contributor Author

dtaylor113 commented Nov 21, 2020 •
edited

@dtaylor113 Do you know if this instance needs to be updated?

https://github.com/openshift/console/blob/master/frontend/packages/dev-console/integration-tests/views/pipeline.view.ts#L9

Nope, that is a protractor test and the ID of the element is still yaml-create, I only updated the (cypress) data-test of the create button.

@dtaylor113
Copy link
Contributor Author

/retest

1 similar comment
@dtaylor113
Copy link
Contributor Author

/retest

spadgett
spadgett approved these changes Nov 23, 2020
View reviewed changes
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@spadgett
Copy link
Member

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 23, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett

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

@dtaylor113
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 8b3508d into openshift:master Nov 23, 2020
@dtaylor113 dtaylor113 deleted the add-i18n-testing-to-crud-tests branch November 25, 2020 14:20
@spadgett spadgett added this to the v4.7 milestone Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spadgett spadgett spadgett approved these changes

@rhamilto rhamilto Awaiting requested review from rhamilto

@rebeccaalpert rebeccaalpert Awaiting requested review from rebeccaalpert

@jhadvig jhadvig Awaiting requested review from jhadvig

@TheRealJon TheRealJon Awaiting requested review from TheRealJon

Assignees

@spadgett spadgett

@dtaylor113 dtaylor113

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/olm Related to OLM lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.7
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dtaylor113 @spadgett @openshift-ci-robot @openshift-bot @openshift-merge-robot