Skip to content

Commit

Permalink
Lantern: remove ts-nochecks from tests
Browse files Browse the repository at this point in the history
- Simulator
- ConnectionPool
- DNS Cache
- BaseNode

Bug: 348449529
Change-Id: I4ca7c72ceccdabbcdd1dc21f899cdf81d25f5c2d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6167550
Auto-Submit: Jack Franklin <[email protected]>
Commit-Queue: Jack Franklin <[email protected]>
Reviewed-by: Connor Clark <[email protected]>
  • Loading branch information
jackfranklin authored and Devtools-frontend LUCI CQ committed Jan 13, 2025
1 parent f856b16 commit e93ae5e
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 124 deletions.
111 changes: 59 additions & 52 deletions front_end/models/trace/lantern/graph/BaseNode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,38 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @ts-nocheck TODO(crbug.com/348449529)

import * as Lantern from '../lantern.js';

const {BaseNode, NetworkNode} = Lantern.Graph;

function sortedById(nodeArray: Lantern.Node[]) {
function sortedById(nodeArray: Lantern.Graph.Node[]) {
return nodeArray.sort((node1, node2) => node1.id.localeCompare(node2.id));
}

/**
* In the real implementation we pass around Node<T>, which are either CPUNodes
* or NetworkNodes. Rather than construct those for these tests, which don't
* need real nodes, we instead create BaseNodes and cast them to satisfy TS.
*/
function makeFakeNode(name: string): Lantern.Graph.Node {
return new BaseNode(name) as Lantern.Graph.Node;
}

function createComplexGraph() {
// B F
// / \ /
// A D - E
// \ / \
// C G - H

const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeD = new BaseNode('D');
const nodeE = new BaseNode('E');
const nodeF = new BaseNode('F');
const nodeG = new BaseNode('G');
const nodeH = new BaseNode('H');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');
const nodeD = makeFakeNode('D');
const nodeE = makeFakeNode('E');
const nodeF = makeFakeNode('F');
const nodeG = makeFakeNode('G');
const nodeH = makeFakeNode('H');

nodeA.addDependent(nodeB);
nodeA.addDependent(nodeC);
Expand All @@ -52,15 +59,15 @@ function createComplexGraph() {
describe('BaseNode', () => {
describe('#constructor', () => {
it('should set the ID', () => {
const node = new BaseNode('foo');
const node = makeFakeNode('foo');
assert.strictEqual(node.id, 'foo');
});
});

describe('.addDependent', () => {
it('should add the correct edge', () => {
const nodeA = new BaseNode('1');
const nodeB = new BaseNode('2');
const nodeA = makeFakeNode('1');
const nodeB = makeFakeNode('2');
nodeA.addDependent(nodeB);

assert.deepEqual(nodeA.getDependents(), [nodeB]);
Expand All @@ -70,16 +77,16 @@ describe('BaseNode', () => {

describe('.addDependency', () => {
it('should add the correct edge', () => {
const nodeA = new BaseNode('1');
const nodeB = new BaseNode('2');
const nodeA = makeFakeNode('1');
const nodeB = makeFakeNode('2');
nodeA.addDependency(nodeB);

assert.deepEqual(nodeA.getDependencies(), [nodeB]);
assert.deepEqual(nodeB.getDependents(), [nodeA]);
});

it('throw when trying to add a dependency on itself', () => {
const nodeA = new BaseNode('1');
const nodeA = makeFakeNode('1');
expect(() => nodeA.addDependency(nodeA)).to.throw();
});
});
Expand Down Expand Up @@ -121,8 +128,8 @@ describe('BaseNode', () => {

describe('.cloneWithoutRelationships', () => {
it('should create a copy', () => {
const node = new BaseNode('1');
const neighbor = new BaseNode('2');
const node = makeFakeNode('1');
const neighbor = makeFakeNode('2');
node.addDependency(neighbor);
const clone = node.cloneWithoutRelationships();

Expand All @@ -132,9 +139,9 @@ describe('BaseNode', () => {
});

it('should copy isMainDocument', () => {
const node = new BaseNode('1');
const node = makeFakeNode('1');
node.setIsMainDocument(true);
const networkNode = new NetworkNode({});
const networkNode = new NetworkNode({} as Lantern.Types.NetworkRequest);
networkNode.setIsMainDocument(true);

assert.isOk(node.cloneWithoutRelationships().isMainDocument());
Expand All @@ -144,8 +151,8 @@ describe('BaseNode', () => {

describe('.cloneWithRelationships', () => {
it('should create a copy of a basic graph', () => {
const node = new BaseNode('1');
const neighbor = new BaseNode('2');
const node = makeFakeNode('1');
const neighbor = makeFakeNode('2');
node.addDependency(neighbor);
const clone = node.cloneWithRelationships();

Expand Down Expand Up @@ -188,12 +195,12 @@ describe('BaseNode', () => {
// C - D - E - F
// / \
// A - - - - - - - B
const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeD = new BaseNode('D');
const nodeE = new BaseNode('E');
const nodeF = new BaseNode('F');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');
const nodeD = makeFakeNode('D');
const nodeE = makeFakeNode('E');
const nodeF = makeFakeNode('F');

nodeA.addDependent(nodeB);
nodeF.addDependent(nodeB);
Expand Down Expand Up @@ -238,7 +245,7 @@ describe('BaseNode', () => {
const graph = createComplexGraph();
assert.throws(
// clone from root to nodeB, but called on nodeD
_ => graph.nodeD.cloneWithRelationships(node => node.id === 'B'),
() => graph.nodeD.cloneWithRelationships(node => node.id === 'B'),
/^Cloned graph missing node$/,
);
});
Expand All @@ -247,15 +254,15 @@ describe('BaseNode', () => {
describe('.traverse', () => {
it('should visit every dependent node', () => {
const graph = createComplexGraph();
const ids = [];
const ids: string[] = [];
graph.nodeA.traverse(node => ids.push(node.id));

assert.deepEqual(ids, ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H']);
});

it('should include a shortest traversal path to every dependent node', () => {
const graph = createComplexGraph();
const paths = [];
const paths: string[][] = [];
graph.nodeA.traverse((node, traversalPath) => {
assert.strictEqual(node.id, traversalPath[0].id);
paths.push(traversalPath.map(node => node.id));
Expand All @@ -275,7 +282,7 @@ describe('BaseNode', () => {

it('should respect getNext', () => {
const graph = createComplexGraph();
const ids = [];
const ids: string[] = [];
graph.nodeF.traverse(
node => ids.push(node.id),
node => node.getDependencies(),
Expand All @@ -295,9 +302,9 @@ describe('BaseNode', () => {
// B
// / \
// A - C
const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');

nodeA.addDependent(nodeC);
nodeA.addDependent(nodeB);
Expand All @@ -308,9 +315,9 @@ describe('BaseNode', () => {

it('should return true for basic cycles', () => {
// A - B - C - A!
const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');

nodeA.addDependent(nodeB);
nodeB.addDependent(nodeC);
Expand All @@ -323,9 +330,9 @@ describe('BaseNode', () => {
// A!
// /
// A - B - C
const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');

nodeA.addDependent(nodeB);
nodeB.addDependent(nodeC);
Expand All @@ -338,14 +345,14 @@ describe('BaseNode', () => {
// B - D - F - G - C!
// / /
// A - - C - E - H
const nodeA = new BaseNode('A');
const nodeB = new BaseNode('B');
const nodeC = new BaseNode('C');
const nodeD = new BaseNode('D');
const nodeE = new BaseNode('E');
const nodeF = new BaseNode('F');
const nodeG = new BaseNode('G');
const nodeH = new BaseNode('H');
const nodeA = makeFakeNode('A');
const nodeB = makeFakeNode('B');
const nodeC = makeFakeNode('C');
const nodeD = makeFakeNode('D');
const nodeE = makeFakeNode('E');
const nodeF = makeFakeNode('F');
const nodeG = makeFakeNode('G');
const nodeH = makeFakeNode('H');

nodeA.addDependent(nodeB);
nodeA.addDependent(nodeC);
Expand All @@ -368,11 +375,11 @@ describe('BaseNode', () => {
});

it('works for very large graphs', () => {
const root = new BaseNode('root');
const root = makeFakeNode('root');

let lastNode = root;
for (let i = 0; i < 10000; i++) {
const nextNode = new BaseNode(`child${i}`);
const nextNode = makeFakeNode(`child${i}`);
lastNode.addDependent(nextNode);
lastNode = nextNode;
}
Expand Down
68 changes: 40 additions & 28 deletions front_end/models/trace/lantern/simulation/ConnectionPool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @ts-nocheck TODO(crbug.com/348449529)

import * as Lantern from '../lantern.js';

const {ConnectionPool} = Lantern.Simulation;
Expand All @@ -13,30 +11,39 @@ describe('ConnectionPool', () => {
const throughput = 10000 * 1024;
let requestId: number;

function request(data: Partial<Lantern.NetworkRequest> = {}): Lantern.NetworkRequest {
function request(data: Partial<Lantern.Types.NetworkRequest> = {}): Lantern.Types.NetworkRequest {
const url = data.url || 'http://example.com';
const origin = new URL(url).origin;
const scheme = url.split(':')[0];
const host = new URL(url).hostname;

return {
requestId: String(requestId++),
url,
protocol: 'http/1.1',
parsedURL: {scheme, securityOrigin: origin},
parsedURL: {scheme, securityOrigin: origin, host},
...data,
};
} as Lantern.Types.NetworkRequest;
}

function simulationOptions(options) {
return Object.assign(
{
rtt: 150,
throughput: 1024,
additionalRttByOrigin: new Map(),
serverResponseTimeByOrigin: new Map(),
},
options,
);
function simulationOptions(options: {
rtt?: number,
throughput?: number,
additionalRttByOrigin?: Map<string, number>,
serverResponseTimeByOrigin?: Map<string, number>,
}): Required<Lantern.Types.Simulation.Options> {
const defaults: Required<Lantern.Types.Simulation.Options> = {
rtt: 150,
throughput: 1024,
additionalRttByOrigin: new Map(),
serverResponseTimeByOrigin: new Map(),
// These options are not used by ConnectionPool, but are required in the types.
cpuSlowdownMultiplier: 1,
layoutTaskMultiplier: 1,
observedThroughput: 1,
maximumConcurrentRequests: 8,
};
return Object.assign(defaults, options);
}

beforeEach(() => {
Expand All @@ -47,11 +54,12 @@ describe('ConnectionPool', () => {
it('should create the pool', () => {
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput}));
// Make sure 6 connections are created for each origin
assert.lengthOf(pool.connectionsByOrigin.get('http://example.com'), 6);
assert.lengthOf(pool.connectionsByOrigin.get('http://example.com') ?? [], 6);
// Make sure it populates connectionWasReused
assert.isFalse(pool.connectionReusedByRequestId.get('1'));

const connection = pool.connectionsByOrigin.get('http://example.com')[0];
const connection = pool.connectionsByOrigin.get('http://example.com')?.[0];
assert.isOk(connection);
assert.strictEqual(connection.rtt, rtt);
assert.strictEqual(connection.throughput, throughput);
assert.strictEqual(connection.serverLatency, 30); // sets to default value
Expand All @@ -60,30 +68,32 @@ describe('ConnectionPool', () => {
it('should set TLS properly', () => {
const recordA = request({url: 'https://example.com'});
const pool = new ConnectionPool([recordA], simulationOptions({rtt, throughput}));
const connection = pool.connectionsByOrigin.get('https://example.com')[0];
assert.isOk(connection.ssl, 'should have set connection TLS');
const connection = pool.connectionsByOrigin.get('https://example.com')?.[0];
assert.isOk(connection?.ssl, 'should have set connection TLS');
});

it('should set H2 properly', () => {
const recordA = request({protocol: 'h2'});
const pool = new ConnectionPool([recordA], simulationOptions({rtt, throughput}));
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
assert.isOk(connection.isH2(), 'should have set HTTP/2');
assert.lengthOf(pool.connectionsByOrigin.get('http://example.com'), 1);
const connection = pool.connectionsByOrigin.get('http://example.com')?.[0];
assert.isOk(connection?.isH2(), 'should have set HTTP/2');
assert.lengthOf(pool.connectionsByOrigin.get('http://example.com') ?? [], 1);
});

it('should set origin-specific RTT properly', () => {
const additionalRttByOrigin = new Map([['http://example.com', 63]]);
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput, additionalRttByOrigin}));
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
assert.isOk(connection.rtt, rtt + 63);
const connection = pool.connectionsByOrigin.get('http://example.com')?.[0];
assert.isOk(connection);
assert.strictEqual(connection.rtt, rtt + 63);
});

it('should set origin-specific server latency properly', () => {
const serverResponseTimeByOrigin = new Map([['http://example.com', 63]]);
const pool = new ConnectionPool([request()], simulationOptions({rtt, throughput, serverResponseTimeByOrigin}));
const connection = pool.connectionsByOrigin.get('http://example.com')[0];
assert.isOk(connection.serverLatency, 63);
const connection = pool.connectionsByOrigin.get('http://example.com')?.[0];
assert.isOk(connection);
assert.strictEqual(connection.serverLatency, 63);
});
});

Expand Down Expand Up @@ -129,7 +139,7 @@ describe('ConnectionPool', () => {
assert.isOk(pool.acquire(warmRecord), 'should have acquired connection');
pool.release(coldRecord);

for (const connection of pool.connectionsByOrigin.get('http://example.com')) {
for (const connection of pool.connectionsByOrigin.get('http://example.com') ?? []) {
connection.setWarmed(true);
}

Expand All @@ -146,7 +156,9 @@ describe('ConnectionPool', () => {
pool.connectionReusedByRequestId.set(recordB.requestId, true);
pool.connectionReusedByRequestId.set(recordC.requestId, true);

const [connectionWarm, connectionWarmer, connectionWarmest] = pool.connectionsByOrigin.get('http://example.com');
const connections = pool.connectionsByOrigin.get('http://example.com');
assert.isOk(connections);
const [connectionWarm, connectionWarmer, connectionWarmest] = connections;
connectionWarm.setWarmed(true);
connectionWarm.setCongestionWindow(10);
connectionWarmer.setWarmed(true);
Expand Down
Loading

0 comments on commit e93ae5e

Please sign in to comment.