diff --git a/CHANGELOG.md b/CHANGELOG.md index 13c642f8..83123f8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Fix tests that failed when `SOURCE_DATE_EPOCH` is set in the environment - Add a `skip_schema_validation` option to `parse_objects` to skip schema validation of input files ([bug 2076540](https://bugzilla.mozilla.org/show_bug.cgi?id=2076540)) - Mark `test_logging` as requiring a web connection +- BUGFIX: Ensure ALL_OBJECT_CHECKS actually run against the known items ([bug 2088071](https://bugzilla.mozilla.org/show_bug.cgi?id=2088071)) +- Reserve "tags" and "pings" as ping names. These cannot be used for custom pings ## 22.0.1 diff --git a/glean_parser/lint.py b/glean_parser/lint.py index cafb5731..5ed0ba9f 100644 --- a/glean_parser/lint.py +++ b/glean_parser/lint.py @@ -641,11 +641,20 @@ def _lint_all_objects( ) -> List[GlinterNit]: nits: List[GlinterNit] = [] - pings = objects.get("pings") + pings = {} + metrics = {} + + for key, item in objects.items(): + if key == "pings": + pings.update(item) + elif key == "tags": + pass + else: + metrics.update(item) + if not pings: return [] - metrics = objects.get("all_metrics") if not metrics: return [] diff --git a/glean_parser/pings.py b/glean_parser/pings.py index 937bb48f..163ff52b 100644 --- a/glean_parser/pings.py +++ b/glean_parser/pings.py @@ -14,7 +14,17 @@ from . import util -RESERVED_PING_NAMES = ["baseline", "metrics", "events", "deletion-request", "default"] +RESERVED_PING_NAMES = [ + "baseline", + "metrics", + "events", + "deletion-request", + "default", + "server-deletion-request", + "tags", + "pings", + "health", +] class Ping: diff --git a/tests/data/reserved_pings.yamlx b/tests/data/reserved_pings.yamlx new file mode 100644 index 00000000..cab3739c --- /dev/null +++ b/tests/data/reserved_pings.yamlx @@ -0,0 +1,27 @@ +# Any copyright is dedicated to the Public Domain. +# https://creativecommons.org/publicdomain/zero/1.0/ + +--- +$schema: moz://mozilla.org/schemas/glean/pings/2-0-0 + +# A list of all reserved ping names. +# This is verified in a test to match the list of defined reserved pings. + +metrics: &common + description: A ping + include_client_id: true + bugs: + - https://example.com/bugs + data_reviews: + - https://example.com/reviews + notification_emails: + - CHANGE-ME@test-only.com + +baseline: *common +events: *common +deletion-request: *common +default: *common +server-deletion-request: *common +tags: *common +pings: *common +health: *common diff --git a/tests/data/unknown_ping_used.yaml b/tests/data/unknown_ping_used.yaml index 66c98c95..75780f28 100644 --- a/tests/data/unknown_ping_used.yaml +++ b/tests/data/unknown_ping_used.yaml @@ -25,6 +25,8 @@ all_metrics: send_in_pings: - metrics - events + no_lint: + - METRIC_ON_EVENTS_LIFETIME non_existent_ping: <<: *defaults diff --git a/tests/test_lint.py b/tests/test_lint.py index f81fcccb..46f3786b 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -505,7 +505,7 @@ def test_unknown_pings_lint(): assert len(errs) == 0 nits = lint.lint_metrics(all_objects.value, parser_config={}) - assert len(nits) == 2 + assert len(nits) == 1 assert nits[0].check_name == "UNKNOWN_PING_REFERENCED" assert nits[0].name == "all_metrics.non_existent_ping" assert "does-not-exist" in nits[0].msg diff --git a/tests/test_parser.py b/tests/test_parser.py index 7b9bec40..947100f8 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -12,6 +12,7 @@ import pytest from glean_parser import metrics +from glean_parser import pings from glean_parser import parser import util @@ -1637,3 +1638,12 @@ def test_categories(): errs = list(all_metrics) assert len(errs) == 0 + + +def test_reserved_pings(): + all_metrics = parser.parse_objects(ROOT / "data" / "reserved_pings.yamlx") + + errors = list(all_metrics) + assert len(errors) == len(pings.RESERVED_PING_NAMES) + for error in errors: + assert "Ping uses a reserved name" in error