From a45f1021108bff51ec562ddbca8571a10423e15d Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Wed, 8 Jan 2025 08:23:57 -0400 Subject: [PATCH 1/2] remove feature flag from commit page --- .../CommitBundleAnalysis.test.tsx | 93 ++++++++----------- .../CommitBundleAnalysis.tsx | 9 +- 2 files changed, 39 insertions(+), 63 deletions(-) diff --git a/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.test.tsx b/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.test.tsx index e5c8ca19fd..20ea75f47a 100644 --- a/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.test.tsx +++ b/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.test.tsx @@ -11,14 +11,6 @@ import { MemoryRouter, Route } from 'react-router-dom' import CommitBundleAnalysis from './CommitBundleAnalysis' -const mocks = vi.hoisted(() => ({ - useFlags: vi.fn(), -})) - -vi.mock('shared/featureFlags', () => ({ - useFlags: mocks.useFlags, -})) - vi.mock('./CommitBundleAnalysisTable', () => ({ default: () =>
CommitBundleAnalysisTable
, })) @@ -190,7 +182,6 @@ interface SetupArgs { firstPullRequest?: boolean comparisonError?: boolean hasCachedBundle?: boolean - featureFlag?: boolean } describe('CommitBundleAnalysis', () => { @@ -203,7 +194,6 @@ describe('CommitBundleAnalysis', () => { firstPullRequest = false, comparisonError = false, hasCachedBundle = false, - featureFlag = false, }: SetupArgs = { coverageEnabled: true, bundleAnalysisEnabled: true, @@ -212,11 +202,8 @@ describe('CommitBundleAnalysis', () => { firstPullRequest: false, comparisonError: false, hasCachedBundle: false, - featureFlag: false, } ) { - mocks.useFlags.mockReturnValue({ displayCachedBundleBanner: featureFlag }) - server.use( graphql.query('CommitPageData', () => { return HttpResponse.json({ @@ -259,32 +246,30 @@ describe('CommitBundleAnalysis', () => { expect(commitBundleAnalysisTable).toBeInTheDocument() }) - describe('feature flag is enabled', () => { - describe('there are cached bundles', () => { - it('renders CachedBundleContentBanner', async () => { - setup({ featureFlag: true, hasCachedBundle: true }) - render(, { wrapper }) + describe('there are cached bundles', () => { + it('renders CachedBundleContentBanner', async () => { + setup({ hasCachedBundle: true }) + render(, { wrapper }) - const cachedBundleContentBanner = await screen.findByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).toBeInTheDocument() - }) + const cachedBundleContentBanner = await screen.findByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).toBeInTheDocument() }) + }) - describe('there are no cached bundles', () => { - it('does not render CachedBundleContentBanner', async () => { - setup({ featureFlag: true, hasCachedBundle: false }) - render(, { wrapper }) + describe('there are no cached bundles', () => { + it('does not render CachedBundleContentBanner', async () => { + setup({ hasCachedBundle: false }) + render(, { wrapper }) - await waitFor(() => queryClientV5.isFetching()) - await waitFor(() => !queryClientV5.isFetching()) + await waitFor(() => queryClientV5.isFetching()) + await waitFor(() => !queryClientV5.isFetching()) - const cachedBundleContentBanner = screen.queryByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).not.toBeInTheDocument() - }) + const cachedBundleContentBanner = screen.queryByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).not.toBeInTheDocument() }) }) @@ -562,32 +547,30 @@ describe('CommitBundleAnalysis', () => { }) }) - describe('feature flag is enabled', () => { - describe('there are cached bundles', () => { - it('renders CachedBundleContentBanner', async () => { - setup({ featureFlag: true, hasCachedBundle: true }) - render(, { wrapper }) + describe('there are cached bundles', () => { + it('renders CachedBundleContentBanner', async () => { + setup({ hasCachedBundle: true }) + render(, { wrapper }) - const cachedBundleContentBanner = await screen.findByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).toBeInTheDocument() - }) + const cachedBundleContentBanner = await screen.findByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).toBeInTheDocument() }) + }) - describe('there are no cached bundles', () => { - it('does not render CachedBundleContentBanner', async () => { - setup({ featureFlag: true, hasCachedBundle: false }) - render(, { wrapper }) + describe('there are no cached bundles', () => { + it('does not render CachedBundleContentBanner', async () => { + setup({ hasCachedBundle: false }) + render(, { wrapper }) - await waitFor(() => queryClientV5.isFetching()) - await waitFor(() => !queryClientV5.isFetching()) + await waitFor(() => queryClientV5.isFetching()) + await waitFor(() => !queryClientV5.isFetching()) - const cachedBundleContentBanner = screen.queryByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).not.toBeInTheDocument() - }) + const cachedBundleContentBanner = screen.queryByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).not.toBeInTheDocument() }) }) diff --git a/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.tsx b/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.tsx index 1996591882..5f8d810d39 100644 --- a/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.tsx +++ b/src/pages/CommitDetailPage/CommitBundleAnalysis/CommitBundleAnalysis.tsx @@ -4,7 +4,6 @@ import { useParams } from 'react-router-dom' import { CachedBundleContentBanner } from 'shared/CachedBundleContentBanner/CachedBundleContentBanner' import ComparisonErrorBanner from 'shared/ComparisonErrorBanner' -import { useFlags } from 'shared/featureFlags' import { ReportUploadType } from 'shared/utils/comparison' import Spinner from 'ui/Spinner' @@ -43,10 +42,6 @@ const BundleContent: React.FC = ({ bundleCompareType, hasCachedBundle, }) => { - const { displayCachedBundleBanner } = useFlags({ - displayCachedBundleBanner: false, - }) - if (bundleCompareType === 'FirstPullRequest') { return ( <> @@ -70,9 +65,7 @@ const BundleContent: React.FC = ({ return ( <> - {hasCachedBundle && displayCachedBundleBanner ? ( - - ) : null} + {hasCachedBundle ? : null} }> From 7f035f60f39c2a2355e904abebb1763c56abd465 Mon Sep 17 00:00:00 2001 From: nicholas-codecov Date: Wed, 8 Jan 2025 08:24:49 -0400 Subject: [PATCH 2/2] remove feature flag from pull page --- .../PullBundleAnalysis.test.tsx | 132 ++++++++---------- .../PullBundleAnalysis/PullBundleAnalysis.tsx | 9 +- 2 files changed, 57 insertions(+), 84 deletions(-) diff --git a/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.test.tsx b/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.test.tsx index baaf4bbea0..162a3ec411 100644 --- a/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.test.tsx +++ b/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.test.tsx @@ -18,14 +18,6 @@ import PullBundleAnalysis from './PullBundleAnalysis' import { TBundleAnalysisComparisonResult } from '../queries/PullPageDataQueryOpts' -const mocks = vi.hoisted(() => ({ - useFlags: vi.fn(), -})) - -vi.mock('shared/featureFlags', () => ({ - useFlags: mocks.useFlags, -})) - vi.mock('./EmptyTable', () => ({ default: () =>
EmptyTable
, })) @@ -159,7 +151,6 @@ interface SetupArgs { coverageEnabled?: boolean bundleAnalysisEnabled?: boolean hasCachedBundle?: boolean - featureFlag?: boolean } describe('PullBundleAnalysis', () => { @@ -169,10 +160,7 @@ describe('PullBundleAnalysis', () => { coverageEnabled = false, bundleAnalysisEnabled = false, hasCachedBundle = false, - featureFlag = false, }: SetupArgs) { - mocks.useFlags.mockReturnValue({ displayCachedBundleBanner: featureFlag }) - server.use( graphql.query('PullPageData', () => { return HttpResponse.json({ @@ -217,42 +205,38 @@ describe('PullBundleAnalysis', () => { expect(table).toBeInTheDocument() }) - describe('feature flag is enabled', () => { - describe('there is a cached bundle', () => { - it('renders the CachedBundleContentBanner', async () => { - setup({ - coverageEnabled: true, - bundleAnalysisEnabled: true, - hasCachedBundle: true, - featureFlag: true, - }) - render(, { wrapper }) - - const cachedBundleContentBanner = await screen.findByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).toBeInTheDocument() + describe('there is a cached bundle', () => { + it('renders the CachedBundleContentBanner', async () => { + setup({ + coverageEnabled: true, + bundleAnalysisEnabled: true, + hasCachedBundle: true, }) + render(, { wrapper }) + + const cachedBundleContentBanner = await screen.findByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).toBeInTheDocument() }) + }) - describe('there is not a cached bundle', () => { - it('does not render the CachedBundleContentBanner', async () => { - setup({ - coverageEnabled: true, - bundleAnalysisEnabled: true, - hasCachedBundle: false, - featureFlag: true, - }) - render(, { wrapper }) - - await waitFor(() => queryClientV5.isFetching()) - await waitFor(() => !queryClientV5.isFetching()) - - const cachedBundleContentBanner = screen.queryByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).not.toBeInTheDocument() + describe('there is not a cached bundle', () => { + it('does not render the CachedBundleContentBanner', async () => { + setup({ + coverageEnabled: true, + bundleAnalysisEnabled: true, + hasCachedBundle: false, }) + render(, { wrapper }) + + await waitFor(() => queryClientV5.isFetching()) + await waitFor(() => !queryClientV5.isFetching()) + + const cachedBundleContentBanner = screen.queryByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).not.toBeInTheDocument() }) }) }) @@ -371,42 +355,38 @@ describe('PullBundleAnalysis', () => { expect(table).toBeInTheDocument() }) - describe('feature flag is enabled', () => { - describe('there is a cached bundle', () => { - it('renders the CachedBundleContentBanner', async () => { - setup({ - coverageEnabled: false, - bundleAnalysisEnabled: true, - hasCachedBundle: true, - featureFlag: true, - }) - render(, { wrapper }) - - const cachedBundleContentBanner = await screen.findByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).toBeInTheDocument() + describe('there is a cached bundle', () => { + it('renders the CachedBundleContentBanner', async () => { + setup({ + coverageEnabled: false, + bundleAnalysisEnabled: true, + hasCachedBundle: true, }) + render(, { wrapper }) + + const cachedBundleContentBanner = await screen.findByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).toBeInTheDocument() }) + }) - describe('there is not a cached bundle', () => { - it('does not render the CachedBundleContentBanner', async () => { - setup({ - coverageEnabled: false, - bundleAnalysisEnabled: true, - hasCachedBundle: false, - featureFlag: true, - }) - render(, { wrapper }) - - await waitFor(() => queryClientV5.isFetching()) - await waitFor(() => !queryClientV5.isFetching()) - - const cachedBundleContentBanner = screen.queryByText( - 'The reported bundle size includes cached data from previous commits' - ) - expect(cachedBundleContentBanner).not.toBeInTheDocument() + describe('there is not a cached bundle', () => { + it('does not render the CachedBundleContentBanner', async () => { + setup({ + coverageEnabled: false, + bundleAnalysisEnabled: true, + hasCachedBundle: false, }) + render(, { wrapper }) + + await waitFor(() => queryClientV5.isFetching()) + await waitFor(() => !queryClientV5.isFetching()) + + const cachedBundleContentBanner = screen.queryByText( + 'The reported bundle size includes cached data from previous commits' + ) + expect(cachedBundleContentBanner).not.toBeInTheDocument() }) }) }) diff --git a/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.tsx b/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.tsx index 1ebbf48ca7..bb75482074 100644 --- a/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.tsx +++ b/src/pages/PullRequestPage/PullBundleAnalysis/PullBundleAnalysis.tsx @@ -4,7 +4,6 @@ import { useParams } from 'react-router-dom' import { CachedBundleContentBanner } from 'shared/CachedBundleContentBanner/CachedBundleContentBanner' import ComparisonErrorBanner from 'shared/ComparisonErrorBanner' -import { useFlags } from 'shared/featureFlags' import { ReportUploadType } from 'shared/utils/comparison' import Spinner from 'ui/Spinner' @@ -46,10 +45,6 @@ const BundleContent: React.FC = ({ headHasBundle, hasCachedBundle, }) => { - const { displayCachedBundleBanner } = useFlags({ - displayCachedBundleBanner: false, - }) - if (bundleCompareType === 'FirstPullRequest') { return ( <> @@ -85,9 +80,7 @@ const BundleContent: React.FC = ({ return ( <> - {hasCachedBundle && displayCachedBundleBanner ? ( - - ) : null} + {hasCachedBundle ? : null} }>