Skip to content

Commit

Permalink
Alerting/fix flaky instance test (#58994)
Browse files Browse the repository at this point in the history
The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.
  • Loading branch information
gmmorris authored Mar 11, 2020
1 parent 1525dbe commit fa16b2b
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('alert_details', () => {
).containsMatchingElement(
<EuiTitle size="m">
<h1>
{alert.name}
<span>{alert.name}</span>
&emsp;
<EuiBetaBadge
label="Beta"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ export const AlertDetails: React.FunctionComponent<AlertDetailsProps> = ({
<EuiPageContentHeader>
<EuiPageContentHeaderSection>
<EuiTitle size="m">
<h1 data-test-subj="alertDetailsTitle">
{alert.name}
<h1>
<span data-test-subj="alertDetailsTitle">{alert.name}</span>
&emsp;
<EuiBetaBadge
label="Beta"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,18 @@ describe('alert_instances', () => {

const alertState = mockAlertState();
const instances: AlertInstanceListItem[] = [
alertInstanceToListItem(alert, 'first_instance', alertState.alertInstances!.first_instance),
alertInstanceToListItem(alert, 'second_instance', alertState.alertInstances!.second_instance),
alertInstanceToListItem(
fakeNow.getTime(),
alert,
'first_instance',
alertState.alertInstances!.first_instance
),
alertInstanceToListItem(
fakeNow.getTime(),
alert,
'second_instance',
alertState.alertInstances!.second_instance
),
];

expect(
Expand All @@ -48,6 +58,24 @@ describe('alert_instances', () => {
).toEqual(instances);
});

it('render a hidden field with duration epoch', () => {
const alert = mockAlert();
const alertState = mockAlertState();

expect(
shallow(
<AlertInstances
durationEpoch={fake2MinutesAgo.getTime()}
{...mockAPIs}
alert={alert}
alertState={alertState}
/>
)
.find('[name="alertInstancesDurationEpoch"]')
.prop('value')
).toEqual(fake2MinutesAgo.getTime());
});

it('render all active alert instances', () => {
const alert = mockAlert();
const instances = {
Expand Down Expand Up @@ -75,8 +103,8 @@ describe('alert_instances', () => {
.find(EuiBasicTable)
.prop('items')
).toEqual([
alertInstanceToListItem(alert, 'us-central', instances['us-central']),
alertInstanceToListItem(alert, 'us-east', instances['us-east']),
alertInstanceToListItem(fakeNow.getTime(), alert, 'us-central', instances['us-central']),
alertInstanceToListItem(fakeNow.getTime(), alert, 'us-east', instances['us-east']),
]);
});

Expand All @@ -98,8 +126,8 @@ describe('alert_instances', () => {
.find(EuiBasicTable)
.prop('items')
).toEqual([
alertInstanceToListItem(alert, 'us-west'),
alertInstanceToListItem(alert, 'us-east'),
alertInstanceToListItem(fakeNow.getTime(), alert, 'us-west'),
alertInstanceToListItem(fakeNow.getTime(), alert, 'us-east'),
]);
});
});
Expand All @@ -117,7 +145,7 @@ describe('alertInstanceToListItem', () => {
},
};

expect(alertInstanceToListItem(alert, 'id', instance)).toEqual({
expect(alertInstanceToListItem(fakeNow.getTime(), alert, 'id', instance)).toEqual({
instance: 'id',
status: { label: 'Active', healthColor: 'primary' },
start,
Expand All @@ -140,7 +168,7 @@ describe('alertInstanceToListItem', () => {
},
};

expect(alertInstanceToListItem(alert, 'id', instance)).toEqual({
expect(alertInstanceToListItem(fakeNow.getTime(), alert, 'id', instance)).toEqual({
instance: 'id',
status: { label: 'Active', healthColor: 'primary' },
start,
Expand All @@ -153,7 +181,7 @@ describe('alertInstanceToListItem', () => {
const alert = mockAlert();
const instance: RawAlertInstance = {};

expect(alertInstanceToListItem(alert, 'id', instance)).toEqual({
expect(alertInstanceToListItem(fakeNow.getTime(), alert, 'id', instance)).toEqual({
instance: 'id',
status: { label: 'Active', healthColor: 'primary' },
start: undefined,
Expand All @@ -168,7 +196,7 @@ describe('alertInstanceToListItem', () => {
meta: {},
};

expect(alertInstanceToListItem(alert, 'id', instance)).toEqual({
expect(alertInstanceToListItem(fakeNow.getTime(), alert, 'id', instance)).toEqual({
instance: 'id',
status: { label: 'Active', healthColor: 'primary' },
start: undefined,
Expand All @@ -181,7 +209,7 @@ describe('alertInstanceToListItem', () => {
const alert = mockAlert({
mutedInstanceIds: ['id'],
});
expect(alertInstanceToListItem(alert, 'id')).toEqual({
expect(alertInstanceToListItem(fakeNow.getTime(), alert, 'id')).toEqual({
instance: 'id',
status: { label: 'Inactive', healthColor: 'subdued' },
start: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type AlertInstancesProps = {
alert: Alert;
alertState: AlertTaskState;
requestRefresh: () => Promise<void>;
durationEpoch?: number;
} & Pick<AlertApis, 'muteAlertInstance' | 'unmuteAlertInstance'>;

export const alertInstancesTableColumns = (
Expand Down Expand Up @@ -134,6 +135,7 @@ export function AlertInstances({
muteAlertInstance,
unmuteAlertInstance,
requestRefresh,
durationEpoch = Date.now(),
}: AlertInstancesProps) {
const [pagination, setPagination] = useState<Pagination>({
index: 0,
Expand All @@ -142,10 +144,10 @@ export function AlertInstances({

const mergedAlertInstances = [
...Object.entries(alertInstances).map(([instanceId, instance]) =>
alertInstanceToListItem(alert, instanceId, instance)
alertInstanceToListItem(durationEpoch, alert, instanceId, instance)
),
...difference(alert.mutedInstanceIds, Object.keys(alertInstances)).map(instanceId =>
alertInstanceToListItem(alert, instanceId)
alertInstanceToListItem(durationEpoch, alert, instanceId)
),
];
const pageOfAlertInstances = getPage(mergedAlertInstances, pagination);
Expand All @@ -158,25 +160,33 @@ export function AlertInstances({
};

return (
<EuiBasicTable
items={pageOfAlertInstances}
pagination={{
pageIndex: pagination.index,
pageSize: pagination.size,
totalItemCount: mergedAlertInstances.length,
}}
onChange={({ page: changedPage }: { page: Pagination }) => {
setPagination(changedPage);
}}
rowProps={() => ({
'data-test-subj': 'alert-instance-row',
})}
cellProps={() => ({
'data-test-subj': 'cell',
})}
columns={alertInstancesTableColumns(onMuteAction)}
data-test-subj="alertInstancesList"
/>
<Fragment>
<input
type="hidden"
data-test-subj="alertInstancesDurationEpoch"
name="alertInstancesDurationEpoch"
value={durationEpoch}
/>
<EuiBasicTable
items={pageOfAlertInstances}
pagination={{
pageIndex: pagination.index,
pageSize: pagination.size,
totalItemCount: mergedAlertInstances.length,
}}
onChange={({ page: changedPage }: { page: Pagination }) => {
setPagination(changedPage);
}}
rowProps={() => ({
'data-test-subj': 'alert-instance-row',
})}
cellProps={() => ({
'data-test-subj': 'cell',
})}
columns={alertInstancesTableColumns(onMuteAction)}
data-test-subj="alertInstancesList"
/>
</Fragment>
);
}
export const AlertInstancesWithApi = withBulkAlertOperations(AlertInstances);
Expand Down Expand Up @@ -207,9 +217,11 @@ const INACTIVE_LABEL = i18n.translate(
{ defaultMessage: 'Inactive' }
);

const durationSince = (start?: Date) => (start ? Date.now() - start.getTime() : 0);
const durationSince = (durationEpoch: number, startTime?: number) =>
startTime ? durationEpoch - startTime : 0;

export function alertInstanceToListItem(
durationEpoch: number,
alert: Alert,
instanceId: string,
instance?: RawAlertInstance
Expand All @@ -221,7 +233,10 @@ export function alertInstanceToListItem(
? { label: ACTIVE_LABEL, healthColor: 'primary' }
: { label: INACTIVE_LABEL, healthColor: 'subdued' },
start: instance?.meta?.lastScheduledActions?.date,
duration: durationSince(instance?.meta?.lastScheduledActions?.date),
duration: durationSince(
durationEpoch,
instance?.meta?.lastScheduledActions?.date?.getTime() ?? 0
),
isMuted,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
const alerting = getService('alerting');
const retry = getService('retry');

// FLAKY: https://github.com/elastic/kibana/issues/57426
describe.skip('Alert Details', function() {
describe('Alert Details', function() {
describe('Header', function() {
const testRunUuid = uuid.v4();
before(async () => {
Expand Down Expand Up @@ -206,8 +205,6 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});

it('renders the active alert instances', async () => {
const testBeganAt = moment().utc();

// Verify content
await testSubjects.existOrFail('alertInstancesList');

Expand All @@ -219,30 +216,42 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
meta: {
lastScheduledActions: { date },
},
}) => moment(date).utc()
}) => date
);

log.debug(`API RESULT: ${JSON.stringify(dateOnAllInstances)}`);

const instancesList = await pageObjects.alertDetailsUI.getAlertInstancesList();
expect(instancesList.map(instance => omit(instance, 'duration'))).to.eql([
{
instance: 'us-central',
status: 'Active',
start: dateOnAllInstances['us-central'].format('D MMM YYYY @ HH:mm:ss'),
start: moment(dateOnAllInstances['us-central'])
.utc()
.format('D MMM YYYY @ HH:mm:ss'),
},
{
instance: 'us-east',
status: 'Active',
start: dateOnAllInstances['us-east'].format('D MMM YYYY @ HH:mm:ss'),
start: moment(dateOnAllInstances['us-east'])
.utc()
.format('D MMM YYYY @ HH:mm:ss'),
},
{
instance: 'us-west',
status: 'Active',
start: dateOnAllInstances['us-west'].format('D MMM YYYY @ HH:mm:ss'),
start: moment(dateOnAllInstances['us-west'])
.utc()
.format('D MMM YYYY @ HH:mm:ss'),
},
]);

const durationEpoch = moment(
await pageObjects.alertDetailsUI.getAlertInstanceDurationEpoch()
).utc();

const durationFromInstanceTillPageLoad = mapValues(dateOnAllInstances, date =>
moment.duration(testBeganAt.diff(moment(date).utc()))
moment.duration(durationEpoch.diff(moment(date).utc()))
);
instancesList
.map(alertInstance => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ export function AlertDetailsPageProvider({ getService }: FtrProviderContext) {
};
});
},
async getAlertInstanceDurationEpoch(): Promise<number> {
const alertInstancesDurationEpoch = await find.byCssSelector(
'input[data-test-subj="alertInstancesDurationEpoch"]'
);
return parseInt(await alertInstancesDurationEpoch.getAttribute('value'), 10);
},
async clickAlertInstanceMuteButton(instance: string) {
const muteAlertInstanceButton = await testSubjects.find(
`muteAlertInstanceButton_${instance}`
Expand Down

0 comments on commit fa16b2b

Please sign in to comment.