Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

Commit

Permalink
HARP-12868: Fix Tile and TileGeometryLoader tests (#1963)
Browse files Browse the repository at this point in the history 
* HARP-12868: Make broken TileGeometryLoader tests fail.

Failing tests weren't either returning a promise or calling done()
callback, therefore error was not reported by mocha.

* HARP-12868: Fix failing TileGeometryLoader and Tile tests.

* HARP-12868: Fix other tests and disable error logging.

* HARP-12868: Address review comments.
  • Loading branch information
@atomicsulfate
atomicsulfate committed Nov 11, 2020
1 parent 99f5028 commit 40a856b
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 95 deletions.
155 changes: 61 additions & 94 deletions @here/harp-mapview-decoder/test/TileGeometryLoaderTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,20 +139,16 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(1);
mapView.taskQueue.processNext(TileTaskGroups.CREATE);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
await geometryLoader.waitFinished().should.be.fulfilled;
expect(geometryLoader.isFinished).to.be.true;
});

it("should not start geometry loading for empty tile", async function() {
geometryLoader!.update();

expect(geometryLoader.hasDecodedTile).to.be.false;
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(0);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
expect(geometryLoader.isFinished).to.be.false;
});
});

Expand Down Expand Up @@ -181,9 +177,8 @@ describe("TileGeometryLoader", function() {
expect(spySetDecodedTile.callCount).equal(1);
expect(spyProcessTechniques.callCount).equal(1);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
await geometryLoader.waitFinished().should.be.fulfilled;
expect(geometryLoader.isFinished).to.be.true;
});

it("should create geometry for decoded tile only once (via taskqueue)", async function() {
Expand All @@ -207,11 +202,10 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});
await geometryLoader.waitFinished().should.be.fulfilled;
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});

it("should not create geometry for invisible tile", async function() {
Expand All @@ -230,11 +224,10 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
});
await geometryLoader.waitFinished().should.be.rejected;
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
});

it("should not create geometry for disposed tile ", async function() {
Expand All @@ -254,11 +247,10 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
});
await geometryLoader.waitFinished().should.be.rejected;
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
});

it("should create geometry for tile which was invisible but now visible", async function() {
Expand All @@ -278,28 +270,28 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(0);

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
// The geometry loader doesn't finish, because we expect the task to expire
expect(geometryLoader.isFinished).to.be.false;
});
await geometryLoader.waitFinished().should.be.rejected;

tile.isVisible = true;
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
// The geometry loader doesn't finish, because we expect the task to expire
expect(geometryLoader.isFinished).to.be.false;

tile.isVisible = true;
geometryLoader.reset();
geometryLoader!.update();

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1, "should create geometry now");
expect(geometryLoader.isFinished).to.be.true;
});
await geometryLoader.waitFinished().should.be.fulfilled;

expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1, "should create geometry now");
expect(geometryLoader.isFinished).to.be.true;
});

it("should reload geometry for already loaded tile that was reset", async function() {
it("should reload geometry for loaded tile that was reset (invalidated)", async function() {
tile.decodedTile = createFakeDecodedTile();

const geometryCreator = TileGeometryCreator.instance;
Expand All @@ -313,57 +305,26 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;

geometryLoader.reset();
expect(geometryLoader.isFinished).to.be.false;
await geometryLoader.waitFinished().should.be.fulfilled;

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(2);
expect(geometryLoader.isFinished).to.be.true;
});
});
});
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;

it("should load geometry for canceled tile that was reset", async function() {
// Simulate a reload (e.g. due to a dirty/invalidated tile), loading a new decoded tile.
geometryLoader.reset();
tile.decodedTile = createFakeDecodedTile();
geometryLoader.update();

const geometryCreator = TileGeometryCreator.instance;
const spyProcessTechniques = sandbox.spy(geometryCreator, "processTechniques") as any;
const spyCreateGeometries = sandbox.spy(geometryCreator, "createAllGeometries") as any;
expect(spyCreateGeometries.callCount).equal(0);
expect(spyProcessTechniques.callCount).equal(0);

geometryLoader!.update();
expect(geometryLoader.isFinished).to.be.false;
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);

tile.isVisible = false;

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;

geometryLoader.reset();
await geometryLoader.waitFinished().should.be.fulfilled;

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});
});
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(2);
expect(geometryLoader.isFinished).to.be.true;
});

it("should load geometry for disposed tile that was reset", async function() {
Expand All @@ -380,22 +341,28 @@ describe("TileGeometryLoader", function() {

tile.dispose();

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
// Dispose should have rejected the promise.
await geometryLoader.waitFinished().should.be.rejected;

// Wait for the geometry creation task to return without creating any geometry.
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).to.be.true;

expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;

geometryLoader.reset();
geometryLoader.update();

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.reset();
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1);

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);
await geometryLoader.waitFinished().should.be.fulfilled;

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});
});
expect(geometryLoader.isFinished).to.be.true;
});
});
});
10 changes: 10 additions & 0 deletions @here/harp-mapview-decoder/test/TileLoaderTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,28 @@ describe("TileLoader", function() {
let mapView: MapView;
let dataSource: DataSource;
let dataProvider: MockDataProvider;
let loggerWasEnabled = true;

before(function() {
tileKey = TileKey.fromRowColumnLevel(0, 0, 0);
mapView = createMockMapView();
dataSource = new MockDataSource();
dataSource.attach(mapView);
const logger = LoggerManager.instance.getLogger("BaseTileLoader");
if (logger) {
loggerWasEnabled = logger.enabled;
logger.enabled = false;
}
});

beforeEach(function() {
dataProvider = new MockDataProvider();
});

after(function() {
LoggerManager.instance.enable("BaseTileLoader", loggerWasEnabled);
});

describe("loadAndDecode()", function() {
it("should load tiles", function() {
const tileLoader = new TileLoader(
Expand Down
1 change: 1 addition & 0 deletions @here/harp-mapview/test/MapViewTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,7 @@ describe("MapView", function() {
getTilingScheme() {
return webMercatorTilingScheme;
},
setLanguages() {},
mapView: undefined
} as any;
fakeElevationRangeSource = {
Expand Down
3 changes: 2 additions & 1 deletion @here/harp-mapview/test/TileTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ describe("Tile", function() {
groups: [TileTaskGroups.CREATE, TileTaskGroups.FETCH_AND_DECODE]
}),
projection: mercatorProjection,
frameNumber: 1
frameNumber: 1,
visibleTileSet: { disposeTile: () => {} } as any
} as MapView;
stubDataSource.attach(mapView);
});
Expand Down
1 change: 1 addition & 0 deletions @here/harp-vectortile-datasource/test/MapViewPickingTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ describe("MapView Picking", async function() {
getTilingScheme() {
return webMercatorTilingScheme;
},
setLanguages() {},
mapView: undefined
} as any;
fakeElevationRangeSource = {
Expand Down
14 changes: 14 additions & 0 deletions @here/harp-webtile-datasource/test/WebTileLoaderTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { TileKey, webMercatorProjection } from "@here/harp-geoutils";
import { CopyrightInfo, MapView, Tile, TileLoaderState } from "@here/harp-mapview";
import { LoggerManager } from "@here/harp-utils";
import * as chai from "chai";
import * as chaiAsPromised from "chai-as-promised";
import * as sinon from "sinon";
Expand All @@ -32,6 +33,15 @@ describe("WebTileLoader", function() {
const dataProvider: WebTileDataProvider = { getTexture: getTextureStub } as any;
let dataSource: WebTileDataSource;
let tile: Tile;
let loggerWasEnabled = true;

before(function() {
const logger = LoggerManager.instance.getLogger("BaseTileLoader");
if (logger) {
loggerWasEnabled = logger.enabled;
logger.enabled = false;
}
});

beforeEach(function() {
dataSource = new WebTileDataSource({
Expand All @@ -43,6 +53,10 @@ describe("WebTileLoader", function() {
getTextureStub.resolves([texture, copyRightInfo]);
});

after(function() {
LoggerManager.instance.enable("BaseTileLoader", loggerWasEnabled);
});

describe("loadAndDecode()", function() {
it("should load textured mesh and copyright info", function() {
const tileLoader = new WebTileLoader(dataSource, tile, dataProvider);
Expand Down
11 changes: 11 additions & 0 deletions @here/harp-webtile-datasource/test/WebTileTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { mercatorProjection, TileKey } from "@here/harp-geoutils";
import { CopyrightInfo, MapView, Tile } from "@here/harp-mapview";
import { TileGeometryCreator } from "@here/harp-mapview/lib/geometry/TileGeometryCreator";
import { LoggerManager } from "@here/harp-utils";
import { expect } from "chai";
import * as sinon from "sinon";

Expand Down Expand Up @@ -82,6 +83,14 @@ describe("WebTileDataSource", function() {
});

it("# disposed tile for rejected Promise", async function() {
const logger = LoggerManager.instance.getLogger("BaseTileLoader");
let loggerWasEnabled = false;

if (logger) {
loggerWasEnabled = logger.enabled;
logger.enabled = false;
}

const noTextureProvider = {
getTexture: sinon.spy((tile: Tile) => {
return Promise.reject();
Expand All @@ -99,6 +108,8 @@ describe("WebTileDataSource", function() {
await tile.load();
expect(fakeWebTileProvider.getTexture.calledOnceWith(tile));
expect(tile.disposed).to.be.true;

LoggerManager.instance.enable("BaseTileLoader", loggerWasEnabled);
});

it("#createWebTileDataSource with renderingOptions opacity", async function() {
Expand Down

0 comments on commit 40a856b

Please sign in to comment.