Skip to content

ApplicationGroupResource unit tests #3851

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
Merged
openshift-merge-robot merged 1 commit into from
Jan 21, 2020
Merged

ApplicationGroupResource unit tests #3851

openshift-merge-robot merged 1 commit into from
Jan 21, 2020

Conversation

@sahil143
Copy link
Contributor

@sahil143 sahil143 commented Jan 2, 2020

ODC-task: https://issues.redhat.com/browse/ODC-2645

This PR adds unit tests for ApplicationGroupResource.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 2, 2020
@openshift-ci-robot openshift-ci-robot added the component/dev-console Related to dev-console label Jan 2, 2020
@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

/kind cleanup

@openshift-ci-robot openshift-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jan 2, 2020
@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

/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 Jan 2, 2020
@andrewballantyne
Copy link
Contributor

andrewballantyne commented Jan 2, 2020

/hold cancel
/retest
#3847 has fixed the e2e tests

@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 Jan 2, 2020
debsmita1
debsmita1 reviewed Jan 6, 2020
View reviewed changes
...end/packages/dev-console/src/components/topology/__tests__/ApplicationGroupResource.spec.tsx Outdated
Copy link
Contributor

@debsmita1 debsmita1 Jan 6, 2020

Choose a reason for hiding this comment

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

The testcase title looks incomplete

...end/packages/dev-console/src/components/topology/__tests__/ApplicationGroupResource.spec.tsx Outdated
Copy link
Contributor

@debsmita1 debsmita1 Jan 6, 2020

Choose a reason for hiding this comment

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

testcase title looks incomplete

...end/packages/dev-console/src/components/topology/__tests__/ApplicationGroupResource.spec.tsx Outdated
Copy link
Contributor

@debsmita1 debsmita1 Jan 6, 2020

Choose a reason for hiding this comment

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

This will always be true. Check this out enzymejs/enzyme#1864

andrewballantyne
andrewballantyne suggested changes Jan 6, 2020
View reviewed changes
Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

/assign

...onsole/src/components/topology/application-panel/__tests__/ApplicationGroupResource.spec.tsx Outdated
Comment on lines 11 to 18
Copy link
Contributor

@andrewballantyne andrewballantyne Jan 6, 2020

Choose a reason for hiding this comment

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

#3851 (comment)

Use .isEmptyRender() equal to false to showcase it didn't render an empty husk.

Copy link
Contributor Author

@sahil143 sahil143 Jan 8, 2020

Choose a reason for hiding this comment

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

fixed

...onsole/src/components/topology/application-panel/__tests__/ApplicationGroupResource.spec.tsx Outdated
Copy link
Contributor

@andrewballantyne andrewballantyne Jan 6, 2020

Choose a reason for hiding this comment

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

Same comment as above, but inverse it:

Suggested change
expect(wrapper.find('div').exists()).toBe(false);
expect(wrapper.isEmptyRender()).toBe(true);

It should be noted you should also try to avoid markup like div as if we ever change it to be something else it'll mess with this test.

Copy link
Contributor

@andrewballantyne andrewballantyne Jan 6, 2020

Choose a reason for hiding this comment

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

I noticed this may not work because of the fragment... not sure why there is a fragment in the first place either. So you'll have to remove the fragment logic and use this above test or find a different way to test without markup being part of your test.

Copy link
Contributor Author

@sahil143 sahil143 Jan 8, 2020

Choose a reason for hiding this comment

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

fixed

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 8, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 8, 2020
christianvogt
christianvogt reviewed Jan 9, 2020
View reviewed changes
.../packages/dev-console/src/components/topology/application-panel/ApplicationGroupResource.tsx Outdated
Comment on lines 21 to 22
Copy link
Contributor

@christianvogt christianvogt Jan 9, 2020

Choose a reason for hiding this comment

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

You should move the file in one commit before making changes so that it doesn't show up as a new file.

Furthermore you can simplify this component by removing the return statement since you're already making other changes.

Copy link
Contributor Author

@sahil143 sahil143 Jan 14, 2020

Choose a reason for hiding this comment

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

simplified the component.

@sahil143
add unit tests for ApplicationGroupResource
37d30e7 
refactor code

refactor TopologyApplicationResource

use isEmptyRender instead of exists

simplify component
@debsmita1
Copy link
Contributor

debsmita1 commented Jan 17, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 17, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, debsmita1, sahil143

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2020

/retest

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

3 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

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

27 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 20, 2020

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 4a46730 into openshift:master Jan 21, 2020
@spadgett spadgett added this to the v4.4 milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@divyanshiGupta divyanshiGupta Awaiting requested review from divyanshiGupta

3 more reviewers

@christianvogt christianvogt christianvogt left review comments

@debsmita1 debsmita1 debsmita1 left review comments

@andrewballantyne andrewballantyne andrewballantyne approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@andrewballantyne andrewballantyne

@debsmita1 debsmita1

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/dev-console Related to dev-console kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

v4.4

Development

Successfully merging this pull request may close these issues.

8 participants

@sahil143 @andrewballantyne @debsmita1 @openshift-ci-robot @openshift-bot @christianvogt @spadgett @openshift-merge-robot