Bug 1872981: Add basic functionality tests for selecting multiple storage classes #6394

afreen23 · 2020-08-21T05:23:15Z

bipuladh · 2020-08-21T05:37:20Z

/retest
/approve

bipuladh · 2020-08-25T05:58:00Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+    portability: null,
+    devicesCount: null,
+  };
+  beforeAll(init);


Please create a anon code block and put the code inside beforeAll it makes things easier for people to understand,.

bipuladh · 2020-08-25T05:58:11Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+    devicesCount: null,
+  };
+  beforeAll(init);
+  afterAll(teardown);


Same as above

bipuladh · 2020-08-25T05:59:44Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+import { NS } from '../../utils/consts';
+import { appHost } from '@console/internal-integration-tests/protractor.conf';
+
+const fetchStorageClusterYAML = () => {


Suggested change

const fetchStorageClusterYAML = () => {

const fetchStorageClusterJSON: K8sResourceKind = () => {

It's not fetching YAML.

bipuladh · 2020-08-25T06:01:42Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+    let index: number;
+    let deviceSets: DeviceSet[];
+    beforeAll(async () => {
+      const yaml: K8sResourceKind = fetchStorageClusterYAML();


Suggested change

const yaml: K8sResourceKind = fetchStorageClusterYAML();

const yaml: K8sResourceKind = fetchStorageClusterJSON();

bipuladh · 2020-08-25T06:02:39Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+      const yaml: K8sResourceKind = fetchStorageClusterYAML();
+      beforeCapacityAddition.deviceSets = yaml.spec.storageDeviceSets.length;
+      await addCapacity(yaml.metadata.uid, scName);
+      const latestYaml: K8sResourceKind = fetchStorageClusterYAML();


Suggested change

const latestYaml: K8sResourceKind = fetchStorageClusterYAML();

const latestYaml: K8sResourceKind = fetchStorageClusterJSON();

bipuladh · 2020-08-25T06:03:19Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+    let latestDeviceSets: DeviceSet[];
+    let latestIndex: number;
+    beforeAll(async () => {
+      const yaml: K8sResourceKind = fetchStorageClusterYAML();


Suggested change

const yaml: K8sResourceKind = fetchStorageClusterYAML();

const json: K8sResourceKind = fetchStorageClusterJSON();

bipuladh · 2020-08-25T06:04:37Z

frontend/packages/ceph-storage-plugin/integration-tests/mocks/storage-class.ts

@@ -1,10 +1,16 @@
-export const storageClass = {
+export const mockNoProvisionerSC = {


Suggested change

export const mockNoProvisionerSC = {

export const NoProvisionerSC = {

No need to add mock in front as it's all inside the mock folder.

When I see mockNoProvisionerSC somewhere I can make out that it is a mock storage class.

Let's follow the pattern that we already have set.

Renamed the test classes to be prefixed with "test" .

bipuladh · 2020-08-25T06:04:44Z

frontend/packages/ceph-storage-plugin/integration-tests/mocks/storage-class.ts

  provisioner: 'kubernetes.io/no-provisioner',
  reclaimPolicy: 'Delete',
 };
+
+export const mockSC = {


Same as above.

bipuladh · 2020-08-25T06:05:23Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+  clickKebabAction,
+} from '../../views/add-capacity.view';
+import { NS } from '../../utils/consts';
+import { appHost } from '@console/internal-integration-tests/protractor.conf';


Nit: move it above relative imports.

bipuladh · 2020-08-25T06:06:55Z

...-storage-plugin/integration-tests/tests/3-tests/multiple-storage-class-selection.scenario.ts

+  const options: ExecFileOptionsWithStringEncoding = {
+    encoding: 'utf8',
+  };


Not required default is utf8

Default is 'Buffer'. https://nodejs.org/api/child_process.html#child_process_child_process_execsync_command_optio

openshift-ci-robot · 2020-08-27T04:39:22Z

@afreen23: This pull request references Bugzilla bug 1872981, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1872981: Add scenarios for multiple storage class selection during add capacity

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.

bipuladh

Please rename the title. Scenarios is not clear.

bipuladh · 2020-08-27T05:11:18Z

frontend/packages/ceph-storage-plugin/integration-tests/mocks/storage-class.ts

-    name: 'test-sc',
-    namespace: 'openshift-storage',
-  },
+  metadata: { name: 'mock-no-prov-sc' },


Suggested change

metadata: { name: 'mock-no-prov-sc' },

metadata: { name: 'no-prov-sc' },

Lets have test-no-prov-sc

bipuladh · 2020-08-27T05:11:28Z

frontend/packages/ceph-storage-plugin/integration-tests/mocks/storage-class.ts

+export const mockSC = {
+  apiVersion: 'storage.k8s.io/v1',
+  kind: 'StorageClass',
+  metadata: { name: 'mock-ebs-sc' },


Suggested change

metadata: { name: 'mock-ebs-sc' },

metadata: { name: 'ebs-sc' },

Lets have test-ebs-sc to differentiate it as a mock stoarge class for testing pupose

afreen23 · 2020-09-08T06:25:04Z

/hold
looking into failing tests

afreen23 · 2020-09-08T07:40:06Z

/hold cancel

…asses Signed-off-by: Afreen Rahman <afrahman@redhat.com>

bipuladh · 2020-09-08T08:26:37Z

/lgtm

openshift-ci-robot · 2020-09-08T08:26:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afreen23, bipuladh

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:

~~frontend/packages/ceph-storage-plugin/OWNERS~~ [afreen23,bipuladh]

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

openshift-bot · 2020-09-08T09:10:54Z

/retest

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

openshift-bot · 2020-09-08T09:23:55Z

/retest

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

openshift-ci-robot · 2020-09-08T12:23:17Z

@afreen23: All pull requests linked via external trackers have merged:

openshift/console#6394

Bugzilla bug 1872981 has been moved to the MODIFIED state.

In response to this:

Bug 1872981: Add basic functionality tests for selecting multiple storage classes

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.

openshift-ci-robot requested review from gnehapk and rawagner August 21, 2020 05:23

openshift-ci-robot added component/ceph Related to ceph-storage-plugin approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 21, 2020

afreen23 force-pushed the expand-tests-fix branch from dd65e48 to 7962c04 Compare August 21, 2020 07:43

openshift-ci-robot added the component/lso Related to local-storage-operator-plugin label Aug 21, 2020

afreen23 force-pushed the expand-tests-fix branch from 7962c04 to a087540 Compare August 21, 2020 08:15

bipuladh suggested changes Aug 25, 2020

View reviewed changes

afreen23 changed the title ~~Add tests for multiple storage class selection to improve the stability of add capacity feature~~ Bug 1872981: Add scenarios for multiple storage class selection during add capacity Aug 27, 2020

openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Aug 27, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 27, 2020

bipuladh reviewed Aug 27, 2020

View reviewed changes

afreen23 force-pushed the expand-tests-fix branch from a087540 to e19a3f2 Compare September 8, 2020 06:16

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 8, 2020

afreen23 force-pushed the expand-tests-fix branch from e19a3f2 to 2ec8c61 Compare September 8, 2020 07:33

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 8, 2020

test: Add basic functionality tests for selecting multiple storage cl…

5ad3d67

…asses Signed-off-by: Afreen Rahman <afrahman@redhat.com>

afreen23 force-pushed the expand-tests-fix branch from 2ec8c61 to 5ad3d67 Compare September 8, 2020 07:45

afreen23 changed the title ~~Bug 1872981: Add scenarios for multiple storage class selection during add capacity~~ Bug 1872981: Add basic functionality tests for selecting multiple storage classes Sep 8, 2020

openshift-ci-robot assigned bipuladh Sep 8, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2020

openshift-merge-robot merged commit 34f5a8b into openshift:master Sep 8, 2020

spadgett added this to the v4.6 milestone Sep 11, 2020

Bug 1872981: Add basic functionality tests for selecting multiple storage classes #6394

Bug 1872981: Add basic functionality tests for selecting multiple storage classes #6394

afreen23 commented Aug 21, 2020

bipuladh commented Aug 21, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

afreen23 Sep 8, 2020

bipuladh Aug 25, 2020

afreen23 Sep 8, 2020

bipuladh Aug 25, 2020

afreen23 Aug 25, 2020

bipuladh Aug 27, 2020

afreen23 Sep 8, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

bipuladh Aug 25, 2020

afreen23 Aug 25, 2020

bipuladh Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

bipuladh left a comment

bipuladh Aug 27, 2020

afreen23 Sep 8, 2020

bipuladh Aug 27, 2020

afreen23 Sep 8, 2020

afreen23 commented Sep 8, 2020

afreen23 commented Sep 8, 2020

bipuladh commented Sep 8, 2020

openshift-ci-robot commented Sep 8, 2020

openshift-bot commented Sep 8, 2020

openshift-bot commented Sep 8, 2020

openshift-ci-robot commented Sep 8, 2020

	const fetchStorageClusterYAML = () => {
	const fetchStorageClusterJSON: K8sResourceKind = () => {

	const yaml: K8sResourceKind = fetchStorageClusterYAML();
	const yaml: K8sResourceKind = fetchStorageClusterJSON();

	const latestYaml: K8sResourceKind = fetchStorageClusterYAML();
	const latestYaml: K8sResourceKind = fetchStorageClusterJSON();

	const yaml: K8sResourceKind = fetchStorageClusterYAML();
	const json: K8sResourceKind = fetchStorageClusterJSON();

		@@ -1,10 +1,16 @@
		export const storageClass = {
		export const mockNoProvisionerSC = {

	export const mockNoProvisionerSC = {
	export const NoProvisionerSC = {

	metadata: { name: 'mock-no-prov-sc' },
	metadata: { name: 'no-prov-sc' },

	metadata: { name: 'mock-ebs-sc' },
	metadata: { name: 'ebs-sc' },

Bug 1872981: Add basic functionality tests for selecting multiple storage classes #6394

Bug 1872981: Add basic functionality tests for selecting multiple storage classes #6394

Conversation

afreen23 commented Aug 21, 2020

bipuladh commented Aug 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Aug 27, 2020

bipuladh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afreen23 commented Sep 8, 2020

afreen23 commented Sep 8, 2020

bipuladh commented Sep 8, 2020

openshift-ci-robot commented Sep 8, 2020

openshift-bot commented Sep 8, 2020

openshift-bot commented Sep 8, 2020

openshift-ci-robot commented Sep 8, 2020