Skip to content

Commit

Permalink
[Uptime] [Test] Repurpose unit test assertions to avoid flakiness (#4…
Browse files Browse the repository at this point in the history
…0650) (#42977)

I had previously revised these tests to utilize Jest's toBeCloseTo assertion, because it provides a precision parameter. My understanding when I implemented this test was that it would check n significant digits, so if I ran code like expect(myValue).toBeCloseTo(36000, 3), a value like 36015 would be accepted by the assertion. After seeing this test fail in a flaky way while testing other changes (it failed when myValue === 36001), I researched the function more closely and found that it does not behave this way.

This led me to opt for the current solution, which should help avoid any flakiness issues while continuing to rely on the end-user simplicity of a snapshot. I do not expect a variance greater than 1 when running these tests, so the threshold of 100 should be sufficient to consider these tests stable.
  • Loading branch information
andrewvc committed Sep 4, 2019
1 parent 0676d7e commit 307e002
Showing 1 changed file with 20 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import { DatabaseAdapter } from '../../database';
import { ElasticsearchMonitorsAdapter } from '../elasticsearch_monitors_adapter';
import { CountParams, CountResponse } from 'elasticsearch';

const assertCloseTo = (actual: number, expected: number, precision: number) => {
if (Math.abs(expected - actual) > precision) {
throw new Error(`expected [${actual}] to be within ${precision} of ${actual}`);
}
};

// FIXME: there are many untested functions in this adapter. They should be tested.
describe('ElasticsearchMonitorsAdapter', () => {
let defaultCountResponse: CountResponse;
Expand Down Expand Up @@ -81,7 +87,7 @@ describe('ElasticsearchMonitorsAdapter', () => {
});

it('getMonitorChartsData will run expected parameters when no location is specified', async () => {
expect.assertions(4);
expect.assertions(3);
const searchMock = jest.fn();
const search = searchMock.bind({});
const database = {
Expand All @@ -102,7 +108,12 @@ describe('ElasticsearchMonitorsAdapter', () => {
10
);
expect(fixedInterval).not.toBeNaN();
expect(fixedInterval).toBeCloseTo(36000, 3);

/**
* The value based on the input should be ~36000
*/
assertCloseTo(fixedInterval, 36000, 100);

set(
searchMock.mock.calls[0][1],
'body.aggs.timeseries.date_histogram.fixed_interval',
Expand All @@ -112,7 +123,7 @@ describe('ElasticsearchMonitorsAdapter', () => {
});

it('getMonitorChartsData will provide expected filters when a location is specified', async () => {
expect.assertions(4);
expect.assertions(3);
const searchMock = jest.fn();
const search = searchMock.bind({});
const database = {
Expand All @@ -133,7 +144,12 @@ describe('ElasticsearchMonitorsAdapter', () => {
10
);
expect(fixedInterval).not.toBeNaN();
expect(fixedInterval).toBeCloseTo(36000, 3);

/**
* The value based on the input should be ~36000
*/
assertCloseTo(fixedInterval, 36000, 100);

set(
searchMock.mock.calls[0][1],
'body.aggs.timeseries.date_histogram.fixed_interval',
Expand Down

0 comments on commit 307e002

Please sign in to comment.