HARP 14470 (#2173)

* HARP-14470 code coverage moved to karma

- nyc only supports code coverage in node, however
  we have a number of browser based tests, and it
  is the primary use case, so we have decided to
  do the code coverage in karma.
- the karma.conf.js previously used the
  pre-bundled test bundle, but this doesn't give
  us the code coverage per line number, I could
  have tried to find a way to remap the bundle to
  lines, but because the karma-typescript project
  is well maintained, I went this road. This
  however means there is a lot of configuration to
  get the tests running correctly, especially
  those that need to load resources.
- note also, there are quite a few tests that time
  out when run in the browser, hence the need to
  change the timeout in some cases
- note also, karma outputs the code coverage
  format using lcov, this is different to nyc and
  means that the code coverage drops a lot, this
  is because the lcov format can tell if a line is
  fully covered, i.e. for an if statement, it
  records if both true and false variants are
  executed. As such, lines without full branch
  coverage are treated as not covered, and this
  reduces the code coverage in the codecov.io
  report available via github. The local text
  report which is shown when running `yarn
  cov-test` doesn't have this behaviour, i.e lines
  that have just one branch executed are counted
  as part of the code coverage.

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Change to `coverage` directory

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Fixes when coresdk is installed next to sdk

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Move karma.conf.js to karma.options.js so sdk can import and extend the configuration

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Fix prefixes for the exclude files

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Change script to match sdk, i.e `test-cov`

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Change reporter to use the same as nyc so that the numbers are better to compare

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Remove redundant test files

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Add comment to ticket to refactor code

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Removing incorrect comments

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Readd test script

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Exclude code coverage for vector_tile.js, I think
it is from MapBox, anything ending with node.ts or index*.ts, or anything
which is in a test/ directory

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 HARP-15326 Don't include react components

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

* HARP-14470 Remove test utils for IBCT

Signed-off-by: Jonathan Stichbury <2533428+nzjony@users.noreply.github.com>

.github/workflows/ci.yaml

@@ -53,17 +53,12 @@ jobs:
       run: yarn run pre-test
       shell: bash
     - name: Test Coverage
-      run: yarn cov-test --forbid-only --timeout ${{ env.nodeTestTimeout }}
-      shell: bash
-      if: matrix.os != 'windows-latest'
-    - name: Generate coverage report
-      run: |
-        yarn cov-report-html
+      run: yarn test-cov
       shell: bash
       if: matrix.os != 'windows-latest'
     - name: Publish Coverage (Linux)
       run: |
-        bash <(curl -s https://codecov.io/bash) -f coverage/*.json
+        bash <(curl -s https://codecov.io/bash) -f coverage/**/*.info
       shell: bash
       if: matrix.os == 'ubuntu-latest'
     - name: Save Coverage Report (Linux)

.github/workflows/deploy_to_npm.yaml

@@ -32,8 +32,7 @@ jobs:
             - name: Test
               run: |
                   yarn pre-test
-                  yarn cov-test --forbid-only
-                  yarn build-tests
+                  yarn test-cov
                   yarn karma-headless
                   yarn karma-headless-firefox
                   ./scripts/test-npm-packages.sh

@here/create-harp.gl-app/package.json

@@ -9,9 +9,6 @@
     "files": [
         "src/index.js"
     ],
-    "scripts": {
-        "test": "mocha $EXTRA_MOCHA_ARGS ./test/*.js"
-    },
     "repository": {
         "type": "git",
         "url": "https://github.com/heremaps/harp.gl.git",

@here/harp-debug-datasource/package.json

@@ -5,11 +5,9 @@
     "main": "index.js",
     "typings": "index",
     "directories": {
-        "test": "test",
         "lib": "lib"
     },
     "scripts": {
-        "test": "cross-env mocha --require source-map-support/register $EXTRA_MOCHA_ARGS ./test/*.js",
         "build": "tsc --build $EXTRA_TSC_ARGS",
         "prepare": "cross-env tsc --build $EXTRA_TSC_ARGS"
     },

@here/harp-lines/package.json

@@ -9,7 +9,7 @@
         "lib": "lib"
     },
     "scripts": {
-        "test": "cross-env mocha --require source-map-support/register $EXTRA_MOCHA_ARGS ./test/*.js",
+        "test": "echo \"The testing of this package is handled by the image based comparison tests by the sdk\"",
         "build": "tsc --build $EXTRA_TSC_ARGS",
         "prepare": "cross-env tsc --build $EXTRA_TSC_ARGS"
     },

@here/harp-mapview/test/ImageCacheTest.ts

@@ -73,6 +73,7 @@ describe("MapViewImageCache", function () {
 
     if (typeof document !== "undefined") {
         it("#addImage with load", async function () {
+            this.timeout(5000);
             const cache = new MapViewImageCache();
             cache.clear();
 
@@ -104,6 +105,8 @@ describe("MapViewImageCache", function () {
         });
 
         it("#addImage (load cancelled)", async function () {
+            this.timeout(5000);
+
             const cache = new MapViewImageCache();
             cache.clear();
 
@@ -432,6 +435,7 @@ describe("ImageCache", function () {
         });
 
         it("#loadImage, from bad url", async function () {
+            this.timeout(5000);
             const owner = {};
             const cache = ImageCache.instance;
 

@here/harp-mapview/test/MapViewTest.ts

@@ -1732,12 +1732,10 @@ describe("MapView", function () {
 
     describe("frame complete", function () {
         it("MapView emits frame complete for empty map", async function () {
-            this.timeout(100);
             mapView = new MapView(mapViewOptions);
             return await waitForEvent(mapView, MapViewEventNames.FrameComplete);
         });
         it("MapView emits frame complete after map initialized", async function () {
-            this.timeout(100);
             mapView = new MapView(mapViewOptions);
 
             const dataSource = new FakeOmvDataSource({ name: "omv" });
@@ -1746,7 +1744,6 @@ describe("MapView", function () {
             return await waitForEvent(mapView, MapViewEventNames.FrameComplete);
         });
         it("MapView emits frame complete again after map update", async function () {
-            this.timeout(100);
             mapView = new MapView(mapViewOptions);
 
             const dataSource = new FakeOmvDataSource({ name: "omv" });

@here/harp-test-utils/index.ts → @here/harp-test-utils/index.node.ts

@@ -4,5 +4,5 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-export * from "./lib/TestDataUtils";
+export * from "./lib/TestDataUtils.node";
 export * from "./lib/TestUtils";

@here/harp-test-utils/lib/ProfileHelper.ts

@@ -11,14 +11,15 @@ import * as fs from "fs";
 //
 // `performance`, `PerformanceObserver`, ... are global in Browser environment, but only available
 // from `perf_hooks` in node env.
-//
-if (typeof window === "undefined") {
-    const perfHooks = require("perf_hooks");
-
-    (global as any).performance = perfHooks.performance;
-    (global as any).PerformanceObserver = perfHooks.PerformanceObserver;
-    (global as any).PerformanceEntry = perfHooks.PerformanceEntry;
-}
+// Note, the following had to be commented out, because "karma-typescript" tries to resolve the
+// require, even before executing the if condition, see HARP-15322
+// if (typeof window === "undefined") {
+//     const perfHooks = require("perf_hooks");
+
+//     (global as any).performance = perfHooks.performance;
+//     (global as any).PerformanceObserver = perfHooks.PerformanceObserver;
+//     (global as any).PerformanceEntry = perfHooks.PerformanceEntry;
+// }
 
 export interface PerformanceTestStats {
     min: number;

@here/harp-test-utils/package.json

@@ -2,7 +2,7 @@
     "name": "@here/harp-test-utils",
     "version": "0.24.0",
     "description": "Provides utilities for tests",
-    "main": "index.js",
+    "main": "index.node.js",
     "browser": "index.web.js",
     "typings": "index.web",
     "scripts": {

@here/harp-test-utils/test/TestDataUtilsTest.ts

@@ -8,7 +8,7 @@
 
 import { assert } from "chai";
 
-import { loadTestResource } from "../index";
+import { loadTestResource } from "../index.web";
 
 describe("@here/harp-test-utils", function () {
     describe("#loadTestResource", function () {

@here/harp-utils/index.web.ts → @here/harp-utils/index.node.ts

@@ -5,4 +5,4 @@
  */
 
 export * from "./index-common";
-export * from "./lib/UrlPlatformUtils.web";
+export * from "./lib/UrlPlatformUtils";

@here/harp-utils/lib/UrlPlatformUtils.ts

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { baseUrl } from "./UrlUtils";
+
 /**
  * Get base URL for from where relative URLs will be loaded.
  *
@@ -13,5 +15,8 @@
  * * In node, it resolves to `file://${process.cwd()}`.
  */
 export function getAppBaseUrl() {
-    return `file://${process.cwd()}/`;
+    if (typeof window === "undefined") {
+        return `file://${process.cwd()}/`;
+    }
+    return baseUrl(window.location.href);
 }

@here/harp-utils/package.json

@@ -2,8 +2,8 @@
     "name": "@here/harp-utils",
     "version": "0.24.0",
     "description": "Provides utilities: logging, debugging.",
-    "main": "index.js",
-    "browser": "index.web.js",
+    "main": "index.node.js",
+    "browser": "index.js",
     "scripts": {
         "build": "tsc --build $EXTRA_TSC_ARGS",
         "test": "cross-env mocha --require source-map-support/register $EXTRA_MOCHA_ARGS ./test/*.js",

@here/harp-utils/test/LoggerManagerTest.ts

@@ -105,10 +105,13 @@ describe("LoggerManager", function () {
 
     it("Replace default console channel", function () {
         const manager = LoggerManager.instance;
+        const previousChannel = manager.channel;
         const multiChannel = new MultiChannel();
         LoggerManager.instance.setChannel(multiChannel);
 
         assert.equal(manager.channel instanceof ConsoleChannel, false);
+
+        LoggerManager.instance.setChannel(previousChannel);
     });
 
     describe("Check channel compliancy", function () {

@here/harp-utils/test/WorkerChannelTest.ts

@@ -49,11 +49,16 @@ describe("WorkerChannel", function () {
 
         const stubbedPost = sinon.stub(self, "postMessage") as any;
 
+        const previousChannel = LoggerManager.instance.channel;
+        // !!Messing with the global instance is bad, because it affects other tests.!!
         LoggerManager.instance.setChannel(new WorkerChannel());
         const logger = LoggerManager.instance.create(loggerName);
         logger.log(message1, message2);
 
         assert.equal(stubbedPost.callCount, 1);
         assert.isTrue(stubbedPost.alwaysCalledWithExactly(expectedMessage));
+
+        // !!Messing with the global instance is bad, because it affects other tests.!!
+        LoggerManager.instance.setChannel(previousChannel);
     });
 });

@here/harp-vectortile-datasource/test/MapViewPickingTest.ts

@@ -33,7 +33,7 @@ import {
     getTestResourceUrl,
     silenceLoggingAroundFunction,
     waitForEvent
-} from "@here/harp-test-utils/";
+} from "@here/harp-test-utils";
 import * as TestUtils from "@here/harp-test-utils/lib/WebGLStub";
 import { FontCatalog } from "@here/harp-text-canvas";
 import { getAppBaseUrl } from "@here/harp-utils";
@@ -393,6 +393,7 @@ describe("MapView Picking", async function () {
 
         for (const testCase of testCases) {
             it(testCase.name, async () => {
+                this.timeout(10000);
                 mapView.projection = testCase.projection;
 
                 if (testCase.pixelRatio !== undefined) {

karma.conf.js

@@ -11,30 +11,9 @@ const { options } = require("./karma.options");
 /**
  * @param {import("karma").Config} config
  */
-module.exports = function(config) {
+("use.strict");
+module.exports = function (config) {
     config.set({
-        ...options,
-
-        // list of files / patterns to load in the browser
-        files: [
-            "dist/test/three.min.js",
-            "test/browser-resources.js",
-            "dist/test/test.bundle.js",
-            {
-                pattern: "@here/**/*.*",
-                watched: false,
-                included: false
-            },
-            {
-                pattern: "**/harp-fontcatalog/resources/**/*.json",
-                watched: false,
-                included: false
-            }
-        ],
-
-        proxies: {
-            "/browser/@here": "/base/@here",
-            "/browser/@here/harp-fontcatalog/": "/base/node_modules/@here/harp-fontcatalog/"
-        }
+        ...options(config.coverage, false, "")
     });
 };