Skip to content
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

test: Attempt to fix DBaaS unit test flake #11332

Merged
merged 3 commits into from
Nov 26, 2024
Merged

test: Attempt to fix DBaaS unit test flake #11332

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
05b6826
fix test flakes
bnussman Nov 26, 2024
4c7e78c
fix test flakes
bnussman Nov 26, 2024
8bd7428
add changeset
bnussman Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 11 additions & 26 deletions ...es/manager/src/features/Databases/DatabaseDetail/DatabaseBackups/DatabaseBackups.test.tsx
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

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

I just tried to simplify this test as much as possible by removing some waitFors and finds. The updated test still does pretty much the same thing (except asset the loading state).

Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { waitFor } from '@testing-library/react';
import * as React from 'react';

import {
Expand All @@ -18,6 +17,7 @@ describe('Database Backups', () => {
const mockDatabase = databaseFactory.build({
platform: 'rdbms-legacy',
});

const backups = databaseBackupFactory.buildList(7);

server.use(
Expand All @@ -32,34 +32,19 @@ describe('Database Backups', () => {
})
);

const { findAllByText, getByText, queryByText } = renderWithTheme(
<DatabaseBackups />
);
const { findByText, getAllByRole } = renderWithTheme(<DatabaseBackups />);

// Wait for loading to disappear
await waitFor(() =>
expect(queryByText(/loading/i)).not.toBeInTheDocument()
);
for (const backup of backups) {
const expectedDate = formatDate(backup.created, { timezone: 'utc' });

await waitFor(
async () => {
const renderedBackups = await findAllByText((content) => {
return /\d{4}-\d{2}-\d{2}/.test(content);
});
expect(renderedBackups).toHaveLength(backups.length);
},
{ timeout: 5000 }
);
// eslint-disable-next-line no-await-in-loop
const backupItem = await findByText(expectedDate);

await waitFor(
() => {
backups.forEach((backup) => {
const formattedDate = formatDate(backup.created, { timezone: 'utc' });
expect(getByText(formattedDate)).toBeInTheDocument();
});
},
{ timeout: 5000 }
);
expect(backupItem).toBeVisible();
}

// Verify there is a table row for each backup (and a row for the table header)
expect(getAllByRole('row')).toHaveLength(backups.length + 1);
});

it('should render an empty state if there are no backups', async () => {
Expand Down
252 changes: 131 additions & 121 deletions packages/manager/src/features/Databases/DatabaseDetail/DatabaseBackups/DatabaseBackups.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,132 +128,142 @@ export const DatabaseBackups = (props: Props) => {
setSelectedTime(null);
};

return isDefaultDatabase ? (
Copy link
Member Author

Choose a reason for hiding this comment

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

The only thing I changed here was this ternary. I change it to an early return to make this component more readable.

<Paper style={{ marginTop: 16 }}>
<Typography variant="h2">Summary</Typography>
<StyledTypography>
Databases are automatically backed-up with full daily backups for the
past 14 days, and binary logs recorded continuously. Full backups are
version-specific binary backups, which when combined with binary
logs allow for consistent recovery to a specific point in time (PITR).
</StyledTypography>
<Divider spacingBottom={25} spacingTop={25} />
<Typography variant="h2">Restore a Backup</Typography>
<StyledTypography>
{isDatabasesV2GA ? (
<span>
The newest full backup plus incremental is selected by default. Or,
select any date and time within the last 14 days you want to create
a fork from.
</span>
) : (
<span>
Select a date and time within the last 14 days you want to create a
fork from.
</span>
if (isDefaultDatabase) {
return (
<Paper style={{ marginTop: 16 }}>
<Typography variant="h2">Summary</Typography>
<StyledTypography>
Databases are automatically backed-up with full daily backups for the
past 14 days, and binary logs recorded continuously. Full backups are
version-specific binary backups, which when combined with binary
logs allow for consistent recovery to a specific point in time (PITR).
</StyledTypography>
<Divider spacingBottom={25} spacingTop={25} />
<Typography variant="h2">Restore a Backup</Typography>
<StyledTypography>
{isDatabasesV2GA ? (
<span>
The newest full backup plus incremental is selected by default.
Or, select any date and time within the last 14 days you want to
create a fork from.
</span>
) : (
<span>
Select a date and time within the last 14 days you want to create
a fork from.
</span>
)}
</StyledTypography>
{unableToRestoreCopy && (
<Notice spacingTop={16} text={unableToRestoreCopy} variant="info" />
)}
</StyledTypography>
{unableToRestoreCopy && (
<Notice spacingTop={16} text={unableToRestoreCopy} variant="info" />
)}
{isDatabasesV2GA && (
<RadioGroup
aria-label="type"
name="type"
onChange={handleOnVersionOptionChange}
value={versionOption}
>
<FormControlLabel
control={<Radio />}
data-qa-dbaas-radio="Newest"
disabled={disabled}
label="Newest full backup plus incremental"
value="newest"
/>
<FormControlLabel
control={<Radio />}
data-qa-dbaas-radio="DateTime"
disabled={disabled}
label="Specific date & time"
value="dateTime"
/>
</RadioGroup>
)}
<Grid container justifyContent="flex-start" mt={2}>
<Grid item lg={3} md={4} xs={12}>
<Typography variant="h3">Date</Typography>
<LocalizationProvider dateAdapter={AdapterLuxon}>
<StyledDateCalendar
shouldDisableDate={(date) =>
isDateOutsideBackup(date, oldestBackup?.startOf('day'))
}
disabled={disabled || versionOption === 'newest'}
onChange={handleDateChange}
value={selectedDate}
{isDatabasesV2GA && (
<RadioGroup
aria-label="type"
name="type"
onChange={handleOnVersionOptionChange}
value={versionOption}
>
<FormControlLabel
control={<Radio />}
data-qa-dbaas-radio="Newest"
disabled={disabled}
label="Newest full backup plus incremental"
value="newest"
/>
</LocalizationProvider>
<FormControlLabel
control={<Radio />}
data-qa-dbaas-radio="DateTime"
disabled={disabled}
label="Specific date & time"
value="dateTime"
/>
</RadioGroup>
)}
<Grid container justifyContent="flex-start" mt={2}>
<Grid item lg={3} md={4} xs={12}>
<Typography variant="h3">Date</Typography>
<LocalizationProvider dateAdapter={AdapterLuxon}>
<StyledDateCalendar
shouldDisableDate={(date) =>
isDateOutsideBackup(date, oldestBackup?.startOf('day'))
}
disabled={disabled || versionOption === 'newest'}
onChange={handleDateChange}
value={selectedDate}
/>
</LocalizationProvider>
</Grid>
<Grid item lg={3} md={4} xs={12}>
<Typography variant="h3">Time (UTC)</Typography>
<FormControl style={{ marginTop: 0 }}>
{/* TODO: Replace Time Select to the own custom date-time picker component when it's ready */}
<Autocomplete
getOptionDisabled={(option) =>
isTimeOutsideBackup(
option.value,
selectedDate!,
oldestBackup!
)
}
isOptionEqualToValue={(option, value) =>
option.value === value.value
}
renderOption={(props, option) => {
const { key, ...rest } = props;
return (
<li {...rest} key={key}>
{option.label}
</li>
);
}}
textFieldProps={{
dataAttrs: {
'data-qa-time-select': true,
},
}}
autoComplete={false}
className={classes.timeAutocomplete}
disabled={
disabled || !selectedDate || versionOption === 'newest'
}
label=""
onChange={(_, newTime) => setSelectedTime(newTime)}
options={TIME_OPTIONS}
placeholder="Choose a time"
value={selectedTime}
/>
</FormControl>
</Grid>
</Grid>
<Grid item lg={3} md={4} xs={12}>
<Typography variant="h3">Time (UTC)</Typography>
<FormControl style={{ marginTop: 0 }}>
{/* TODO: Replace Time Select to the own custom date-time picker component when it's ready */}
<Autocomplete
getOptionDisabled={(option) =>
isTimeOutsideBackup(option.value, selectedDate!, oldestBackup!)
}
isOptionEqualToValue={(option, value) =>
option.value === value.value
<Grid item xs={12}>
<Box display="flex" justifyContent="flex-end">
<Button
disabled={
versionOption === 'dateTime' && (!selectedDate || !selectedTime)
}
renderOption={(props, option) => {
const { key, ...rest } = props;
return (
<li {...rest} key={key}>
{option.label}
</li>
);
}}
textFieldProps={{
dataAttrs: {
'data-qa-time-select': true,
},
}}
autoComplete={false}
className={classes.timeAutocomplete}
disabled={disabled || !selectedDate || versionOption === 'newest'}
label=""
onChange={(_, newTime) => setSelectedTime(newTime)}
options={TIME_OPTIONS}
placeholder="Choose a time"
value={selectedTime}
/>
</FormControl>
buttonType="primary"
data-qa-settings-button="restore"
onClick={onRestoreDatabase}
>
Restore
</Button>
</Box>
</Grid>
</Grid>
<Grid item xs={12}>
<Box display="flex" justifyContent="flex-end">
<Button
disabled={
versionOption === 'dateTime' && (!selectedDate || !selectedTime)
}
buttonType="primary"
data-qa-settings-button="restore"
onClick={onRestoreDatabase}
>
Restore
</Button>
</Box>
</Grid>
{database ? (
<DatabaseBackupsDialog
database={database}
onClose={() => setIsRestoreDialogOpen(false)}
open={isRestoreDialogOpen}
selectedDate={selectedDate}
selectedTime={selectedTime?.value}
/>
) : null}
</Paper>
) : (
{database && (
<DatabaseBackupsDialog
database={database}
onClose={() => setIsRestoreDialogOpen(false)}
open={isRestoreDialogOpen}
selectedDate={selectedDate}
selectedTime={selectedTime?.value}
/>
)}
</Paper>
);
}

return (
<DatabaseBackupsLegacy
database={database}
databaseError={databaseError}
Expand Down
13 changes: 6 additions & 7 deletions ...ages/manager/src/features/Databases/DatabaseDetail/DatabaseResize/DatabaseResize.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,16 +395,15 @@ describe('database resize', () => {
},
};

const { getByTestId } = renderWithTheme(
const { findByTestId } = renderWithTheme(
<DatabaseResize database={mockDatabase} />,
{ flags }
);
expect(getByTestId(loadingTestId)).toBeInTheDocument();
await waitForElementToBeRemoved(getByTestId(loadingTestId));
Comment on lines -402 to -403
Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than waiting for the loading state to go away and immediately asserting things, I updated this so that each assertion uses a find. This makes the test more resilient to inconsistent rendering behavior, which is what I suspect is happening in DatabaseResize

expect(getByTestId('database-nodes')).toBeDefined();
expect(getByTestId('database-node-1')).toBeDefined();
expect(getByTestId('database-node-2')).toBeDefined();
expect(getByTestId('database-node-3')).toBeDefined();

expect(await findByTestId('database-nodes')).toBeDefined();
expect(await findByTestId('database-node-1')).toBeDefined();
expect(await findByTestId('database-node-2')).toBeDefined();
expect(await findByTestId('database-node-3')).toBeDefined();
});

it('should disable lower node selections', async () => {
Expand Down