feat(HMS-3332): Migrate to PF5 #365

amirfefer · 2023-12-28T16:49:44Z

This PR migrate to PF5 following the official migration guide and @patternfly/pf-codemods
PF5 supports react 18+, so this PR also update react and react-dom
This PR NOT refactored the Wizard and Select components. These have some heavy breaking changes, and will be refactored in a follow up PR. For now, we use the deprecated components from @patternfly/react-core/deprecated'

@akhil-jha, This might break tests, because some selectors are changed or removed, unit testing is broken a bit, needs a small align.

amirfefer · 2023-12-28T17:32:14Z

This PR also seems to fix the wizard's modal resizing:

Screen.Recording.2023-12-28.at.19.27.53.mov

akhil-jha · 2024-01-02T07:49:15Z

Will you be pushing anymore commits or shall I start testing it? :)

amirfefer · 2024-01-02T18:15:47Z

@akhil-jha, done - unit tests are 🟢

akhil-jha · 2024-01-03T05:52:26Z

Great! I'll start working on it then :)

ezr-ondrej

Code LGTM and I've tested most of the happy paths and empty state for no source and everything works as expected 🎉

Will wait for @akhil-jha to ack this, which might take a while (we need the ephem cluster working first)

akhil-jha · 2024-01-04T15:21:37Z

/retest

ezr-ondrej · 2024-01-04T21:29:03Z

/retest

maaybe now? xD

akhil-jha · 2024-01-05T07:15:39Z

/retest

maaybe now? xD

I am seeing this in the pipeline. Maybe because of this the IQE test isn't starting?

03:04:03 Test Suites: 1 failed, 11 passed, 12 total
03:04:03 Tests:       1 failed, 53 passed, 54 total
03:04:03 Snapshots:   0 total
03:04:03 Time:        48.277 s
03:04:03 Ran all test suites.
03:04:03 ERROR: "test" exited with 1.

lzap · 2024-01-05T10:54:25Z

Can you elaborate? I don’t understand.

ezr-ondrej · 2024-01-05T11:48:05Z

Ah right, I did not even look properly there and missed this!
@amirfefer we are running the unit tests in the pr check as well and it seems to be failing there for some reason and when unit tests fail there, it does not run the pr_check (I'm guessing to safe resources).

This test fails: FAIL src/Components/ProvisioningWizard/ProvisioningWizard.test.js

In the past it was sometimes because of Jenkins being slower then GH actions, so just bumping the limits might help.
If not, we might need to bump adjust the nodejs version we're using in the pipeline, might that have an effect?

ezr-ondrej · 2024-01-05T11:53:12Z

src/Components/ProvisioningWizard/ProvisioningWizard.test.js

@@ -36,17 +36,17 @@ describe('ProvisioningWizard', () => {

      render(<ProvisioningWizard hasAccess image={{ ...awsImage, sourceIDs: ['1'] }} />);
      // wait for the sources to load
-      await screen.findByText('Select account', undefined, { timeout: 10000 });
+      await screen.findByText('Select account', undefined, { timeout: 15000 });


This is the place where the pr_check fails. Let me bump this bit more :)

ezr-ondrej · 2024-01-05T15:15:38Z

/retest

Victoremepunto · 2024-01-05T15:25:26Z

/retest

ezr-ondrej · 2024-01-05T15:32:41Z

/retest

akhil-jha · 2024-01-07T18:08:40Z

src/Components/ProvisioningWizard/steps/ReservationProgress/index.js

+        <EmptyState variant="lg">
+          <EmptyStateHeader
+            headingLevel="h4"
+            title={polledReservation?.success ? 'System(s) launched successfully' : 'Launching system(s)'}


I think it should be titleText. https://www.patternfly.org/components/empty-state#emptystateheader
Because I am getting the following....

oh, good catch! :)

amirfefer · 2024-01-08T17:40:32Z

src/Components/ProvisioningWizard/steps/SourceMissing/index.js

-    <Title size="lg" headingLevel="h4">
-      Loading available Sources
-    </Title>
+    <EmptyStateHeader headingLevel="h4" title="Loading available Sources" icon={<EmptyStateIcon icon={Spinner} />} />


same as here

amirfefer · 2024-01-08T18:53:59Z

src/Components/ProvisioningWizard/ProvisioningWizard.test.js

@@ -36,13 +36,9 @@ describe('ProvisioningWizard', () => {

      render(<ProvisioningWizard hasAccess image={{ ...awsImage, sourceIDs: ['1'] }} />);
      // wait for the sources to load
-      await screen.findByText('Select account', undefined, { timeout: 10000 });


this is redundant check, it just make the test be slower, if the Source 1 exists, we don't need the Select account placeholder.

akhil-jha · 2024-01-08T21:48:49Z

/retest

akhil-jha

commit message check is still failing.

Also, @amirfefer can you restore the pr_check.sh change?
I will be creating a new PR for the CICD_URL change.

amirfefer force-pushed the pf5-migration branch 2 times, most recently from 2544a9d to c058841 Compare January 2, 2024 18:10

ezr-ondrej previously approved these changes Jan 4, 2024

View reviewed changes

ezr-ondrej reviewed Jan 5, 2024

View reviewed changes

ezr-ondrej dismissed their stale review via b17f04c January 5, 2024 11:59

akhil-jha reviewed Jan 7, 2024

View reviewed changes

amirfefer commented Jan 8, 2024

View reviewed changes

amirfefer force-pushed the pf5-migration branch from 321e7fa to bfad349 Compare January 8, 2024 18:52

amirfefer commented Jan 8, 2024

View reviewed changes

akhil-jha previously approved these changes Jan 9, 2024

View reviewed changes

feat(HMS-3332): Migrate to PF5

f2b8c95

ezr-ondrej dismissed akhil-jha’s stale review via f2b8c95 January 9, 2024 09:04

ezr-ondrej force-pushed the pf5-migration branch from bfad349 to f2b8c95 Compare January 9, 2024 09:04

ezr-ondrej approved these changes Jan 9, 2024

View reviewed changes

akhil-jha enabled auto-merge (squash) January 9, 2024 09:16

akhil-jha approved these changes Jan 9, 2024

View reviewed changes

akhil-jha merged commit 295c472 into RHEnVision:main Jan 9, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(HMS-3332): Migrate to PF5 #365

feat(HMS-3332): Migrate to PF5 #365

amirfefer commented Dec 28, 2023

amirfefer commented Dec 28, 2023 •

edited

Loading

akhil-jha commented Jan 2, 2024

amirfefer commented Jan 2, 2024

akhil-jha commented Jan 3, 2024

ezr-ondrej left a comment

akhil-jha commented Jan 4, 2024

ezr-ondrej commented Jan 4, 2024

akhil-jha commented Jan 5, 2024

lzap commented Jan 5, 2024

ezr-ondrej commented Jan 5, 2024

ezr-ondrej Jan 5, 2024

ezr-ondrej commented Jan 5, 2024

Victoremepunto commented Jan 5, 2024

ezr-ondrej commented Jan 5, 2024

akhil-jha Jan 7, 2024

ezr-ondrej Jan 8, 2024

amirfefer Jan 8, 2024

amirfefer Jan 8, 2024 •

edited

Loading

akhil-jha commented Jan 8, 2024

akhil-jha left a comment

feat(HMS-3332): Migrate to PF5 #365

feat(HMS-3332): Migrate to PF5 #365

Conversation

amirfefer commented Dec 28, 2023

amirfefer commented Dec 28, 2023 • edited Loading

akhil-jha commented Jan 2, 2024

amirfefer commented Jan 2, 2024

akhil-jha commented Jan 3, 2024

ezr-ondrej left a comment

Choose a reason for hiding this comment

akhil-jha commented Jan 4, 2024

ezr-ondrej commented Jan 4, 2024

akhil-jha commented Jan 5, 2024

lzap commented Jan 5, 2024

ezr-ondrej commented Jan 5, 2024

ezr-ondrej Jan 5, 2024

Choose a reason for hiding this comment

ezr-ondrej commented Jan 5, 2024

Victoremepunto commented Jan 5, 2024

ezr-ondrej commented Jan 5, 2024

akhil-jha Jan 7, 2024

Choose a reason for hiding this comment

ezr-ondrej Jan 8, 2024

Choose a reason for hiding this comment

amirfefer Jan 8, 2024

Choose a reason for hiding this comment

amirfefer Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

akhil-jha commented Jan 8, 2024

akhil-jha left a comment

Choose a reason for hiding this comment

amirfefer commented Dec 28, 2023 •

edited

Loading

amirfefer Jan 8, 2024 •

edited

Loading