Merge pull request #342 from brizental/1682282-schema-validation

Bug 1682282 - Add tests to validate generated pings against the schema

Author: Beatriz Rizental

.circleci/config.yml

@@ -35,9 +35,9 @@ jobs:
           name: Run circular dependencies checker
           command: npm --prefix ./glean run lint:circular-deps
 
-  unit-tests:
+  test:
     docker:
-      - image: cimg/node:16.1.0
+      - image: cimg/python:3.9.4-node
     steps:
       - checkout
       - run:
@@ -51,7 +51,10 @@ jobs:
           command: npm --prefix ./glean install
       - run:
           name: Run unit tests
-          command: export PATH=.:$PATH && npm --prefix ./glean run test
+          command: export PATH=.:$PATH && npm --prefix ./glean run test:unit
+      - run:
+          name: Run integration tests
+          command: export PATH=.:$PATH && npm --prefix ./glean run test:integration
 
   samples-tests:
     docker:
@@ -196,7 +199,7 @@ workflows:
   ci:
     jobs:
       - lint
-      - unit-tests
+      - test
       - check-qt-js
       - samples-tests
       - docs-deploy:

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 [Full changelog](https://github.com/mozilla/glean.js/compare/v0.14.0...main)
 
+* [#342](https://github.com/mozilla/glean.js/pull/342): BUGFIX: Fix timespan payload representatin to match exactly the payload expected according to the Glean schema.
+
 # v0.14.0 (2021-05-19)
 
 [Full changelog](https://github.com/mozilla/glean.js/compare/v0.13.0...v0.14.0)

bin/parser-for-schema-testing.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+#
+# 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/.
+
+set -eo pipefail
+
+run() {
+    [ "${VERB:-0}" != 0 ] && echo "+ $*"
+    "$@"
+}
+
+# All sed commands below work with either
+# GNU sed (standard on Linux distrubtions) or BSD sed (standard on macOS)
+SED="sed"
+
+WORKSPACE_ROOT="$( cd "$(dirname "$0")/.." ; pwd -P )"
+
+# Generate files using the Glean.js CLI tool (which just runs glean_parser)
+npm run cli -- \
+  translate tests/integration/schema/metrics.yaml tests/integration/schema/pings.yaml \
+  -f typescript \
+  -o tests/integration/schema/generated
+
+# Update metrics import path
+FILE=glean/tests/integration/schema/generated/forTesting.ts
+run $SED -i.bak -E \
+  -e 's#@mozilla/glean/webext/private/metrics#../../../../src/core/metrics/types#g' \
+  "${WORKSPACE_ROOT}/${FILE}"
+run rm "${WORKSPACE_ROOT}/${FILE}.bak"
+
+# Update ping import path
+FILE=glean/tests/integration/schema/generated/pings.ts
+run $SED -i.bak -E \
+ -e 's#@mozilla/glean/webext/private/ping#../../../../src/core/pings/ping_type.js#g' \
+ "${WORKSPACE_ROOT}/${FILE}"
+run rm "${WORKSPACE_ROOT}/${FILE}.bak"
+

docs/adding_a_new_metric_type.md

@@ -171,7 +171,7 @@ async function testGetValue(ping: string = this.sendInPings[0]): Promise<string
 
 ## Testing
 
-Tests for metric type implementations live under the `glean/tests/core/metrics/types` folder. Create a new
+Tests for metric type implementations live under the `glean/tests/unit/core/metrics/types` folder. Create a new
 file with the same name as the one you created in `glean/src/core/metrics/types` to accomodate your
 metric type tests.
 
@@ -181,6 +181,17 @@ Make sure your tests cover at least your metric types basic functionality:
 - The metric correctly reports errors;
 - The metric returns the correct value when it has value.
 
+### Glean schema
+
+It is very important to also check if the new metric payload is valid against the Glean schema.
+Declare a new instance of the metrics you just implemented in `glean/test/integration/schema/metrics.yaml`
+and then make sure you record to that metrics on the `validate generated ping is valid against glean schema`
+test on the `glean/test/integration/schema/schema.spec.ts` test.
+
+This test will fail unless schema changes to accomodate the new metric type have already landed.
+Please see the [mozilla-pipeline-schemas](#mozilla-pipeline-schemas) section on how to add new metric
+types to the Glean schema.
+
 ## Documentation
 
 Glean.js' has linter rules that enforce [JSDoc](https://jsdoc.app/) strings on every public function.

docs/getting_started.md

@@ -28,27 +28,29 @@ npm run test
 ```
 in the `glean` folder.
 
-This runs all the tests in the `glean` folder. 
+This runs all the tests in the `glean` folder.
 Alternatively, you may want to run component-specific tests. 
 To do so, substitute `test` in the command above with one of the followings:
-* `test:core` - executes tests inside `tests/core` folder.
-* `test:plugins` - executes tests inside `tests/plugins` folder.
-* `test:platform` - executes tests inside `tests/platform` folder.
+
+* `test:unit:core` - executes tests inside `tests/unit/core` folder.
+* `test:unit:plugins` - executes tests inside `tests/unit/plugins` folder.
+* `test:unit:platform` - executes tests inside `tests/unit/platform` folder.
+* `test:integration` - executes tests inside `tests/integration` folder.
 
 You might want to run one specific test within a certain folder.
 To run the test with the title `DatetimeMetric` in the `core` folder: 
 ```bash
-npm run test:core -- --grep "DateTimeMetric"
+npm run test:unit:core -- --grep "DateTimeMetric"
 ```
 You should see the output similar to the following:
 ```shell
 
-> @mozilla/[email protected] test:core
-> npm run test:base -- "tests/core/**/*.spec.ts" --recursive "--grep" "DatetimeMetric"
+> @mozilla/[email protected] test:unit:core
+> npm run test:base -- "tests/unit/core/**/*.spec.ts" --recursive "--grep" "DatetimeMetric"
 
 
 > @mozilla/[email protected] test:base
-> node --experimental-modules --experimental-specifier-resolution=node --loader=ts-node/esm node_modules/mocha/lib/cli/cli.js "tests/core/**/*.spec.ts" "--recursive" "--grep" "DatetimeMetric"
+> node --experimental-modules --experimental-specifier-resolution=node --loader=ts-node/esm node_modules/mocha/lib/cli/cli.js "tests/unit/core/**/*.spec.ts" "--recursive" "--grep" "DatetimeMetric"
 
 (node:52217) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
 (Use `node --trace-warnings ...` to show where the warning was created)