Skip to content

Conversation

@cajieh
Copy link
Contributor

@cajieh cajieh commented Aug 28, 2025

No description provided.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 28, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 28, 2025

@cajieh: This pull request references CONSOLE-4598 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

Details

In response to this:

… Library

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 28, 2025
@openshift-ci openshift-ci bot requested review from TheRealJon and spadgett August 28, 2025 15:54
@openshift-ci openshift-ci bot added component/core Related to console core functionality approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 28, 2025
@cajieh cajieh force-pushed the migrate-enzyme-__tests__-components-utils-unit-tests-rtl branch 3 times, most recently from 57ff13e to 09aed35 Compare September 2, 2025 10:46
@cajieh
Copy link
Contributor Author

cajieh commented Sep 2, 2025

/retest

@cajieh cajieh force-pushed the migrate-enzyme-__tests__-components-utils-unit-tests-rtl branch from 09aed35 to d5ae638 Compare September 2, 2025 11:37
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 2, 2025

@cajieh: This pull request references CONSOLE-4598 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

@cajieh
Copy link
Contributor Author

cajieh commented Sep 2, 2025

/test frontend

@cajieh cajieh force-pushed the migrate-enzyme-__tests__-components-utils-unit-tests-rtl branch 4 times, most recently from c397474 to bc783fe Compare September 4, 2025 07:42
@cajieh
Copy link
Contributor Author

cajieh commented Sep 4, 2025

/test frontend

@cajieh cajieh force-pushed the migrate-enzyme-__tests__-components-utils-unit-tests-rtl branch 2 times, most recently from fd20554 to 4b0f530 Compare September 4, 2025 13:45
@cajieh cajieh changed the title [WIP] CONSOLE-4598: Migrate enzyme __tests__/components/utils unit tests to React Testing… CONSOLE-4598: Migrate enzyme __tests__/components/utils unit tests to React Testing… Sep 4, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 4, 2025
Copy link
Member

@logonoff logonoff left a comment

Choose a reason for hiding this comment

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

/label px-approved
/label docs-approved

Comment on lines 7 to 9
jest.mock('../../../components/utils/status-box', () => ({
LoadingBox: () =>
require('react').createElement('div', { 'data-test': 'loading-box' }, 'Loading...'),
}));
Copy link
Member

Choose a reason for hiding this comment

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

i think it would be a bit cleaner to just return a string and detect for this text instead of checking a testid

Suggested change
jest.mock('../../../components/utils/status-box', () => ({
LoadingBox: () =>
require('react').createElement('div', { 'data-test': 'loading-box' }, 'Loading...'),
}));
jest.mock('../../../components/utils/status-box', () => ({
LoadingBox: () => "Loading..."
}));

Comment on lines 8 to 20
jest.mock('react-i18next', () => ({
withTranslation: () => (Component) => {
const WrappedComponent = (props) => {
const t = (key) => key.split('~')[1] || key;
return require('react').createElement(Component, { ...props, t });
};
WrappedComponent.displayName = `withTranslation(${Component.displayName || Component.name})`;
return WrappedComponent;
},
useTranslation: () => ({
t: (key) => key.split('~')[1] || key,
}),
}));
Copy link
Member

Choose a reason for hiding this comment

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

i'd recommend we use the same mocks as we usually do without needing to require react

Suggested change
jest.mock('react-i18next', () => ({
withTranslation: () => (Component) => {
const WrappedComponent = (props) => {
const t = (key) => key.split('~')[1] || key;
return require('react').createElement(Component, { ...props, t });
};
WrappedComponent.displayName = `withTranslation(${Component.displayName || Component.name})`;
return WrappedComponent;
},
useTranslation: () => ({
t: (key) => key.split('~')[1] || key,
}),
}));
jest.mock('react-i18next', () => ({
useTranslation: () => ({
t: (key: string) => key,
}),
withTranslation: () => (Component: any) => Component,
}));

Copy link
Contributor Author

@cajieh cajieh Sep 5, 2025

Choose a reason for hiding this comment

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

Did you try this change? It appears not to work because withTranslation HOC is designed to inject the t function as a prop into the wrapped component.

Copy link
Member

@logonoff logonoff Sep 5, 2025

Choose a reason for hiding this comment

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

I did not try this change with this test suite specifically; however, this mock is already used by several other unit tests

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Sep 5, 2025
@cajieh cajieh force-pushed the migrate-enzyme-__tests__-components-utils-unit-tests-rtl branch from 4b0f530 to 9905587 Compare September 5, 2025 14:24
Copy link
Member

@logonoff logonoff 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 Sep 5, 2025
@cajieh
Copy link
Contributor Author

cajieh commented Sep 5, 2025

QE Approver:
/assign @yapei

@yapei
Copy link
Contributor

yapei commented Sep 8, 2025

/retest-required

@cajieh
Copy link
Contributor Author

cajieh commented Sep 8, 2025

/test frontend

@yapei
Copy link
Contributor

yapei commented Sep 9, 2025

frontend test is still failing....

@cajieh
Copy link
Contributor Author

cajieh commented Sep 9, 2025

/test frontend

import { Map as ImmutableMap } from 'immutable';

export { PodModel } from '../../../models';
export { PodModel, ServiceModel } from '../../../models';
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this changed targeted for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is this changed targeted for?

Oh, that wasn't needed. Removed it. Thanks for the catch!

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2025
@yapei
Copy link
Contributor

yapei commented Sep 10, 2025

/retest-required

1 similar comment
@yapei
Copy link
Contributor

yapei commented Sep 10, 2025

/retest-required

@yapei
Copy link
Contributor

yapei commented Sep 10, 2025

frontend is passing before the last commit
/verified by @yapei

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 10, 2025
@openshift-ci-robot
Copy link
Contributor

@yapei: This PR has been marked as verified by @yapei.

Details

In response to this:

frontend is passing before the last commit
/verified by @yapei

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 openshift-eng/jira-lifecycle-plugin repository.

@logonoff
Copy link
Member

/label qe-approved
/lgtm

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Sep 10, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 10, 2025

@cajieh: This pull request references CONSOLE-4598 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cajieh, logonoff

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-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 343b6dd and 2 for PR HEAD 20fe530 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 10, 2025

@cajieh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn 20fe530 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 05f0ab3 into openshift:main Sep 10, 2025
7 of 8 checks passed
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/core Related to console core functionality docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. 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 verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants