Skip to content

Commit

Permalink
Fix test errors
Browse files Browse the repository at this point in the history
Especially this introduces a specialized Error for symbolication, so
that we can distinguish errors and more easily debug tests by rethrowing
unexpected errors.
  • Loading branch information
julienw committed Mar 7, 2018
1 parent 8bdb24b commit 37d7dad
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 32 deletions.
16 changes: 14 additions & 2 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@
regenerator: true
}
],
"transform-class-properties",
[
"transform-class-properties"
"transform-builtin-extend",
{
globals: ["Error"]
}
]
],
env: {
Expand All @@ -34,7 +38,15 @@
"react",
"flow"
],
plugins: [ "transform-class-properties" ]
plugins: [
"transform-class-properties",
[
"transform-builtin-extend",
{
globals: ["Error"]
}
]
]
}
}
}
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"babel-eslint": "^8.2.1",
"babel-jest": "^22.4.1",
"babel-loader": "^7.1.2",
"babel-plugin-transform-builtin-extend": "^1.1.2",
"babel-plugin-transform-class-properties": "^6.24.1",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-polyfill": "^6.26.0",
Expand Down Expand Up @@ -132,6 +133,7 @@
"\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/src/test/fixtures/mocks/file-mock.js",
"\\.(css|less)$": "<rootDir>/src/test/fixtures/mocks/style-mock.js"
},
"restoreMocks": true,
"setupTestFrameworkScriptFile": "./src/test/setup.js",
"verbose": true
},
Expand Down
3 changes: 3 additions & 0 deletions src/actions/receive-profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ let requestIdleCallbackPolyfill: (

if (typeof window === 'object' && window.requestIdleCallback) {
requestIdleCallbackPolyfill = window.requestIdleCallback;
} else if (typeof process === 'object' && process.nextTick) {
// Node environment
requestIdleCallbackPolyfill = process.nextTick;
} else {
requestIdleCallbackPolyfill = callback => setTimeout(callback, 0);
}
Expand Down
21 changes: 21 additions & 0 deletions src/profile-logic/errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

// @flow

import type { Library } from './symbol-store';

// Used during the symbolication process to express that we couldn't find
// symbols for a specific library
export class SymbolsNotFoundError extends Error {
library: Library;
error: ?Error;

constructor(message: string, library: Library, error?: Error) {
super(message);
this.name = 'SymbolsNotFoundError';
this.library = library;
this.error = error;
}
}
2 changes: 1 addition & 1 deletion src/profile-logic/symbol-store-db-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { SymbolStoreDB } from './symbol-store-db';
import SymbolStoreDB from './symbol-store-db';
import { provideWorkerSide } from '../utils/promise-worker';

provideWorkerSide(self, SymbolStoreDB);
9 changes: 7 additions & 2 deletions src/profile-logic/symbol-store-db.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

// @flow

import { SymbolsNotFoundError } from './errors';

import type {
IDBFactory,
IDBDatabase,
Expand Down Expand Up @@ -38,7 +40,7 @@ const kTwoWeeksInMilliseconds = 2 * 7 * 24 * 60 * 60 * 1000;
* @class SymbolStoreDB
* @classdesc Where does this description show up?
*/
export class SymbolStoreDB {
export default class SymbolStoreDB {
_dbPromise: Promise<IDBDatabase> | null;
_maxCount: number;
_maxAge: number; // in milliseconds
Expand Down Expand Up @@ -196,7 +198,10 @@ export class SymbolStoreDB {
updateDateReq.onsuccess = () => resolve([addrs, index, buffer]);
} else {
reject(
new Error('The requested library does not exist in the database.')
new SymbolsNotFoundError(
'The requested library does not exist in the database.',
{ debugName, breakpadId }
)
);
}
};
Expand Down
23 changes: 17 additions & 6 deletions src/profile-logic/symbol-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

// @flow
import { SymbolStoreDB } from './symbol-store-db';
import SymbolStoreDB from './symbol-store-db';
import { SymbolsNotFoundError } from './errors';
import type { SymbolTableAsTuple } from './symbol-store-db';

type Library = {
export type Library = {
debugName: string,
breakpadId: string,
};
Expand Down Expand Up @@ -57,8 +58,9 @@ export class SymbolStore {

if (this._failedRequests.has(libid)) {
return Promise.reject(
new Error(
"We've tried to request a symbol table for this library before and failed, so we're not trying again."
new SymbolsNotFoundError(
"We've tried to request a symbol table for this library before and failed, so we're not trying again.",
lib
)
);
}
Expand All @@ -74,15 +76,24 @@ export class SymbolStore {
// Try to get the symbol table from the database
const symbolTablePromise = this._db
.getSymbolTable(debugName, breakpadId)
.catch(() => {
.catch(e => {
if (!(e instanceof SymbolsNotFoundError)) {
// rethrow JavaScript programming errors
throw e;
}

// Request the symbol table from the symbol provider.
const symbolTablePromise = this._symbolProvider
.requestSymbolTable(debugName, breakpadId)
.catch(error => {
console.error(`Failed to symbolicate library ${debugName}`, error);
this._failedRequests.add(libid);
this._requestedSymbolTables.delete(libid);
throw error;
throw new SymbolsNotFoundError(
`Failed to symbolicate library ${debugName}`,
lib,
error
);
});

// Once the symbol table comes in, store it in the database, but don't
Expand Down
7 changes: 6 additions & 1 deletion src/profile-logic/symbolication.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import bisection from 'bisection';
import { resourceTypes } from './profile-data';
import { SymbolsNotFoundError } from './errors';

import type {
Profile,
Expand Down Expand Up @@ -270,7 +271,11 @@ async function symbolicateThread(
);
});
})
.catch(() => {
.catch(e => {
if (!(e instanceof SymbolsNotFoundError)) {
// rethrow JavaScript programming error
throw e;
}
// We could not find symbols for this library.
// Don't throw, so that the resulting promise will be resolved, thereby
// indicating that we're done symbolicating with lib.
Expand Down
18 changes: 18 additions & 0 deletions src/test/store/receive-profile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// @flow

import sinon from 'sinon';

import { blankStore } from '../fixtures/stores';
import * as ProfileViewSelectors from '../../reducers/profile-view';
import * as UrlStateSelectors from '../../reducers/url-state';
Expand All @@ -18,6 +19,13 @@ import {
import preprocessedProfile from '../fixtures/profiles/profile-2d-canvas.json';
import getGeckoProfile from '../fixtures/profiles/gecko-profile';

// Mocking SymbolStoreDB
import exampleSymbolTable from '../fixtures/example-symbol-table';
import SymbolStoreDB from '../../profile-logic/symbol-store-db';
jest.mock('../../profile-logic/symbol-store-db');

import { TextDecoder } from 'text-encoding';

describe('actions/receive-profile', function() {
/**
* This function allows to observe all state changes in a Redux store while
Expand Down Expand Up @@ -66,13 +74,23 @@ describe('actions/receive-profile', function() {
getSymbolTable: () => Promise.resolve(),
};
window.geckoProfilerPromise = Promise.resolve(geckoProfiler);

// This is a mock implementation because of the `mock` call above, but
// Flow doesn't know this.
(SymbolStoreDB: any).mockImplementation(() => ({
getSymbolTable: jest.fn().mockResolvedValue(exampleSymbolTable),
}));

window.TextDecoder = TextDecoder;
});

afterEach(function() {
clock.restore();

geckoProfiler = null;
delete window.geckoProfilerPromise;
delete window.TextDecoder;
delete window.requestIdleCallback;
});

it('can retrieve a profile from the addon', async function() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/unit/symbol-store-db.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import 'babel-polyfill';
import { SymbolStoreDB } from '../../profile-logic/symbol-store-db';
import SymbolStoreDB from '../../profile-logic/symbol-store-db';
import exampleSymbolTable from '../fixtures/example-symbol-table';
import fakeIndexedDB from 'fake-indexeddb';
import FDBKeyRange from 'fake-indexeddb/lib/FDBKeyRange';
Expand Down
4 changes: 2 additions & 2 deletions src/types/libdef/npm/babel-jest_vx.x.x.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// flow-typed signature: 28e21cd69ea80cb16ce55bf99f590519
// flow-typed version: <<STUB>>/babel-jest_v^22.4.1/flow_v0.66.0
// flow-typed signature: 20045860fbf63b1b7b655392ef46beeb
// flow-typed version: <<STUB>>/babel-jest_v^20.0.3/flow_v0.66.0

/**
* This is an autogenerated libdef stub for:
Expand Down
42 changes: 27 additions & 15 deletions src/types/libdef/npm/jest_v22.x.x.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// flow-typed signature: 18018da6c1a1d95b4ab1c64bb5fe86ca
// flow-typed version: c1ad61e7d4/jest_v22.x.x/flow_>=v0.39.x

type ExtractPromiseValueFunc = <Value>(Promise<Value>) => Value;
type ExtractPromiseValue<PromiseValue> = $Call<
ExtractPromiseValueFunc,
PromiseValue
>;

type JestMockFn<TArguments: $ReadOnlyArray<*>, TReturn> = {
(...args: TArguments): TReturn,
/**
Expand All @@ -17,7 +23,7 @@ type JestMockFn<TArguments: $ReadOnlyArray<*>, TReturn> = {
* An array that contains all the object instances that have been
* instantiated from this mock function.
*/
instances: Array<TReturn>
instances: Array<TReturn>,
},
/**
* Resets all information stored in the mockFn.mock.calls and
Expand Down Expand Up @@ -66,14 +72,20 @@ type JestMockFn<TArguments: $ReadOnlyArray<*>, TReturn> = {
/**
* Sugar for only returning a value once inside your mock
*/
mockReturnValueOnce(value: TReturn): JestMockFn<TArguments, TReturn>
mockReturnValueOnce(value: TReturn): JestMockFn<TArguments, TReturn>,
mockResolvedValue(
value: ExtractPromiseValue<TReturn>
): JestMockFn<TArguments, TReturn>,
mockResolvedValueOnce(
value: ExtractPromiseValue<TReturn>
): JestMockFn<TArguments, TReturn>,
};

type JestAsymmetricEqualityType = {
/**
* A custom Jasmine equality tester
*/
asymmetricMatch(value: mixed): boolean
asymmetricMatch(value: mixed): boolean,
};

type JestCallsType = {
Expand All @@ -83,19 +95,19 @@ type JestCallsType = {
count(): number,
first(): mixed,
mostRecent(): mixed,
reset(): void
reset(): void,
};

type JestClockType = {
install(): void,
mockDate(date: Date): void,
tick(milliseconds?: number): void,
uninstall(): void
uninstall(): void,
};

type JestMatcherResult = {
message?: string | (() => string),
pass: boolean
pass: boolean,
};

type JestMatcher = (actual: any, expected: any) => JestMatcherResult;
Expand All @@ -110,7 +122,7 @@ type JestPromiseType = {
* Use resolves to unwrap the value of a fulfilled promise so any other
* matcher can be chained. If the promise is rejected the assertion fails.
*/
resolves: JestExpectType
resolves: JestExpectType,
};

/**
Expand Down Expand Up @@ -139,7 +151,7 @@ type EnzymeMatchersType = {
toIncludeText(text: string): void,
toHaveValue(value: any): void,
toMatchElement(element: React$Element<any>): void,
toMatchSelector(selector: string): void
toMatchSelector(selector: string): void,
};

type JestExpectType = {
Expand Down Expand Up @@ -283,7 +295,7 @@ type JestExpectType = {
* Use .toThrowErrorMatchingSnapshot to test that a function throws a error
* matching the most recent snapshot when it is called.
*/
toThrowErrorMatchingSnapshot(): void
toThrowErrorMatchingSnapshot(): void,
};

type JestObjectType = {
Expand Down Expand Up @@ -435,11 +447,11 @@ type JestObjectType = {
* Set the default timeout interval for tests and before/after hooks in milliseconds.
* Note: The default timeout interval is 5 seconds if this method is not called.
*/
setTimeout(timeout: number): JestObjectType
setTimeout(timeout: number): JestObjectType,
};

type JestSpyType = {
calls: JestCallsType
calls: JestCallsType,
};

/** Runs this function after every test inside this context */
Expand Down Expand Up @@ -478,7 +490,7 @@ declare var describe: {
/**
* Skip running this describe block
*/
skip(name: JestTestName, fn: () => void): void
skip(name: JestTestName, fn: () => void): void,
};

/** An individual test unit */
Expand Down Expand Up @@ -530,7 +542,7 @@ declare var it: {
name: JestTestName,
fn?: (done: () => void) => ?Promise<mixed>,
timeout?: number
): void
): void,
};
declare function fit(
name: JestTestName,
Expand Down Expand Up @@ -564,7 +576,7 @@ declare var expect: {
objectContaining(value: Object): void,
/** Matches any received string that contains the exact expected string. */
stringContaining(value: string): void,
stringMatching(value: string | RegExp): void
stringMatching(value: string | RegExp): void,
};

// TODO handle return type
Expand All @@ -590,5 +602,5 @@ declare var jasmine: {
methodNames: Array<string>
): { [methodName: string]: JestSpyType },
objectContaining(value: Object): void,
stringMatching(value: string): void
stringMatching(value: string): void,
};
Loading

0 comments on commit 37d7dad

Please sign in to comment.