fix(snapshot): add test pipelinerun list and capitalize breadcrumbs #847

abhinandan13jan · 2023-10-30T14:52:07Z

Fixes

https://issues.redhat.com/browse/RHTAPBUGS-839

Description

Updates PLR list to include test PLRs with snapshot label
capitalizes snapshot in breadcrumb
capitalizes Environmenr names

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

Chrome
Firefox
Safari
Edge

codecov · 2023-10-30T15:14:11Z

Codecov Report

Merging #847 (c814c03) into main (950ea9e) will increase coverage by 0.07%.
The diff coverage is 81.81%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #847      +/-   ##
==========================================
+ Coverage   85.59%   85.67%   +0.07%     
==========================================
  Files         596      596              
  Lines       15041    15046       +5     
  Branches     4214     4217       +3     
==========================================
+ Hits        12875    12890      +15     
+ Misses       2029     2020       -9     
+ Partials      137      136       -1

Files	Coverage Δ
...SnapshotDetails/EnvironmentProvisionErrorAlert.tsx	`100.00% <100.00%> (ø)`
...mponents/SnapshotDetails/tabs/SnapshotOverview.tsx	`100.00% <100.00%> (+6.06%)`	⬆️
.../SnapshotDetails/tabs/SnapshotPipelineRunsList.tsx	`86.51% <ø> (ø)`
...s/SnapshotDetails/tabs/SnapshotPipelineRunsTab.tsx	`90.62% <100.00%> (+6.75%)`	⬆️
...components/SnapshotDetails/SnapshotDetailsView.tsx	`91.04% <60.00%> (+7.96%)`	⬆️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 950ea9e...c814c03. Read the comment docs.

mattreid · 2023-10-30T18:33:16Z

Is Type being lowercase in the filter selector coming from data, or our code? Other areas we're filtering on PLR type, it shows as capitalized (Build, Test).

abhinandan13jan · 2023-10-31T09:43:22Z

/retest

abhinandan13jan · 2023-10-31T13:10:22Z

@mattreid we get it as lowercase types from the object metadata. I updated to capitalize it while displaying

karthikjeeyar · 2023-11-03T08:28:38Z

src/components/SnapshotDetails/SnapshotDetailsView.tsx

@@ -147,7 +147,7 @@ const SnapshotDetailsView: React.FC<SnapshotDetailsViewProps> = ({
                      <>
                        <StatusIconWithTextLabel
                          key={env.metadata?.name}
-                          text={env.metadata.name}
+                          text={capitalize(env.metadata.name)}


There is a env.spec?.displayName optional field in Environment CR, We must use that if exists or else fallback to the env.metadata.name. This way we will be be respecting the user's choice of displayName rather than Capitalizing the K8s resource name.

Deployment tab uses the above approach, so the Development text comes from spec.displayName and notice the other environment which doesn't have displayName and it uses resource name. Can we stick to this displayName or resource.name (without captializing) approach for consistency ? cc: @mattreid

rottencandy · 2023-11-21T15:36:50Z

src/components/SnapshotDetails/tabs/SnapshotOverview.tsx

@@ -108,7 +109,7 @@ const SnapshotOverviewTab: React.FC<SnapshotOverviewTabProps> = ({
                          <Link
                            to={`/application-pipeline/workspaces/${workspace}/applications/${snapshot.spec.application}/deployments`}
                          >
-                            {env}
+                            {capitalize(env)}


I think we should use environment's spec.displayName here as well. WDYT?

rottencandy

Looking good, but looks like we need more tests to satisfy codecov

openshift-ci · 2023-11-22T08:37:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: abhinandan13jan, rottencandy
Once this PR has been reviewed and has the lgtm label, please assign jrichter1 for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

openshift-ci · 2023-11-22T15:07:03Z

New changes are detected. LGTM label has been removed.

rottencandy · 2023-11-24T07:37:20Z

/retest

fix(snapshot): add test plrs, capitalize breadcrumb and environment

abhinandan13jan · 2023-11-28T08:54:06Z

/retest

openshift-ci · 2023-11-28T09:24:21Z

@abhinandan13jan: all tests passed!

Full PR test history. Your PR dashboard.

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

openshift-ci bot requested review from Katka92 and rottencandy October 30, 2023 14:59

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from c7c149f to b648167 Compare October 30, 2023 15:48

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from b648167 to b78ff9a Compare October 31, 2023 13:07

karthikjeeyar reviewed Nov 3, 2023

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2023

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from b78ff9a to 9d631f8 Compare November 3, 2023 10:50

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 3, 2023

abhinandan13jan requested a review from karthikjeeyar November 7, 2023 08:24

rottencandy reviewed Nov 21, 2023

View reviewed changes

abhinandan13jan requested a review from rottencandy November 21, 2023 17:37

rottencandy approved these changes Nov 22, 2023

View reviewed changes

openshift-ci bot assigned rottencandy Nov 22, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2023

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from a8a3c0b to fbe6316 Compare November 22, 2023 15:05

abhinandan13jan requested a review from rottencandy November 22, 2023 15:06

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 22, 2023

abhinandan13jan force-pushed the fix-snapshot-plr-list branch 5 times, most recently from 13c47d0 to 5939ce9 Compare November 23, 2023 14:13

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from cda0275 to 35c31ca Compare November 24, 2023 13:41

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from 35c31ca to 8172a95 Compare November 24, 2023 17:06

abhinandan13jan added 3 commits November 27, 2023 19:30

fix(snapshots): include plr with both annotation and labels for snapshot

e036b25

fix(snapshot): add test plrs, capitalize breadcrumb and environment

fix(ss): add tests

2858882

feat(snapshots): adding further test suites

c814c03

abhinandan13jan force-pushed the fix-snapshot-plr-list branch from 8172a95 to c814c03 Compare November 27, 2023 14:42

rohitkrai03 merged commit b7359d4 into openshift:main Nov 28, 2023
5 of 7 checks passed

rohitkrai03 mentioned this pull request Dec 1, 2023

chore(main): release rhtap-ui 0.3.0 #871

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(snapshot): add test pipelinerun list and capitalize breadcrumbs #847

fix(snapshot): add test pipelinerun list and capitalize breadcrumbs #847

abhinandan13jan commented Oct 30, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading

mattreid commented Oct 30, 2023

abhinandan13jan commented Oct 31, 2023

abhinandan13jan commented Oct 31, 2023

karthikjeeyar Nov 3, 2023

karthikjeeyar Nov 3, 2023

rottencandy Nov 21, 2023 •

edited

Loading

abhinandan13jan Nov 21, 2023

rottencandy left a comment

openshift-ci bot commented Nov 22, 2023

openshift-ci bot commented Nov 22, 2023

rottencandy commented Nov 24, 2023

abhinandan13jan commented Nov 28, 2023

openshift-ci bot commented Nov 28, 2023

fix(snapshot): add test pipelinerun list and capitalize breadcrumbs #847

fix(snapshot): add test pipelinerun list and capitalize breadcrumbs #847

Conversation

abhinandan13jan commented Oct 30, 2023 • edited Loading

Fixes

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

codecov bot commented Oct 30, 2023 • edited Loading

Codecov Report

mattreid commented Oct 30, 2023

abhinandan13jan commented Oct 31, 2023

abhinandan13jan commented Oct 31, 2023

karthikjeeyar Nov 3, 2023

Choose a reason for hiding this comment

karthikjeeyar Nov 3, 2023

Choose a reason for hiding this comment

rottencandy Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

abhinandan13jan Nov 21, 2023

Choose a reason for hiding this comment

rottencandy left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 22, 2023

openshift-ci bot commented Nov 22, 2023

rottencandy commented Nov 24, 2023

abhinandan13jan commented Nov 28, 2023

openshift-ci bot commented Nov 28, 2023

abhinandan13jan commented Oct 30, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading

rottencandy Nov 21, 2023 •

edited

Loading