1680995 - Add more JSON checks in Gecko Profiler gtests

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P2)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Most profiler gtests only check that some strings are present.
We should be more thorough, and checks that the profile JSON is correct, and contains all expected properties.

And this should make it easier to add tests when adding/modifying/fixing features.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1680995 - Profiler gtests: JSONOutputCheck now parses profile JSON and its properties - r?canaltinova

JSONOutputCheck used to only check the profile output for the presence of some strings.
Now it parses the output as JSON, and navigates the JSON data to check expected properties, including their types, and values as needed.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1680995 - Profiler gtests: Add some general checks - r?canaltinova

JSONOutputCheck now checks a few more properties, including thread contents, and sub-process profiles.

Depends on D98890

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1680995 - Profiler gtests: Use JSONOutputCheck on profiler_get_profile() output - r?canaltinova

Depends on D98891

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1680995 - Profiler gtests: Use JSONOutputCheck in GeckoProfiler.Markers gtest - r?canaltinova

Consolidate all JSON checks to go through JSONOutputCheck.

Depends on D98892

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5af1708f901a
Profiler gtests: JSONOutputCheck now parses profile JSON and its properties - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/ff0701c57f86
Profiler gtests: Add some general checks - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/32045ed29d9e
Profiler gtests: Use JSONOutputCheck on profiler_get_profile() output - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/b802e2d38707
Profiler gtests: Use JSONOutputCheck in GeckoProfiler.Markers gtest - r=canaltinova

Comment 6

[Natalia Csoregi [:nataliaCs]]

Backed out for hazard failures on GeckoProfiler.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/01ac34a546cac054a520a3d8166439e7234c5c33

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&searchStr=linux%2Cx64%2Cdebug%2Chazard-linux64-haz%2Fdebug%2Ch&revision=b802e2d38707bca600ab85c02d70f4a044f9a5ad

failure log: https://treeherder.mozilla.org/logviewer?job_id=324018295&repo=autoland&lineNumber=16247

[task 2020-12-09T08:17:17.079Z] 33:41.31 TEST-PASS | check_textrel | libmozavcodec.so
[task 2020-12-09T08:18:15.425Z] 34:39.65 In file included from Unified_cpp_tests_gtest0.cpp:2:
[task 2020-12-09T08:18:15.425Z] 34:39.65 /builds/worker/checkouts/gecko/tools/profiler/tests/gtest/GeckoProfiler.cpp: In function 'void JSONRootCheck(const Json::Value&, bool)':
[task 2020-12-09T08:18:15.425Z] 34:39.65 /builds/worker/checkouts/gecko/tools/profiler/tests/gtest/GeckoProfiler.cpp:110:21: error: unnecessary parentheses in declaration of 'meta' [-Werror=parentheses]
[task 2020-12-09T08:18:15.425Z] 34:39.65 110 | const Json::Value&(VARIABLE) = (GETTER)
[task 2020-12-09T08:18:15.425Z] 34:39.65 | ^
[task 2020-12-09T08:18:15.425Z] 34:39.65 /builds/worker/checkouts/gecko/tools/profiler/tests/gtest/GeckoProfiler.cpp:172:3: note: in expansion of macro 'GET_JSON'
[task 2020-12-09T08:18:15.425Z] 34:39.65 172 | GET_JSON(meta, aRoot["meta"], Object);
[task 2020-12-09T08:18:15.425Z] 34:39.65 | ^~~~~~~~
[task 2020-12-09T08:18:15.425Z] 34:39.65 /builds/worker/checkouts/gecko/tools/profiler/tests/gtest/GeckoProfiler.cpp:110:21: error: unnecessary parentheses in declaration of 'threads' [-Werror=parentheses]
[task 2020-12-09T08:18:15.425Z] 34:39.65 110 | const Json::Value&(VARIABLE) = (GETTER)
[task 2020-12-09T08:18:15.425Z] 34:39.65 | ^

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you Natalia.

clang-tidy made me do it! (Add parens in the macro.)
I've filed bug 1681464 about the discrepancy between clang-tidy and hazard builds.

Anyway, I think the hazard error is more appropriate in this case, since we're declaring a variable, it wouldn't make sense to provide something that would require parentheses.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6f7e04dd094
Profiler gtests: JSONOutputCheck now parses profile JSON and its properties - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/a38ce531d418
Profiler gtests: Add some general checks - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/cb22dee08955
Profiler gtests: Use JSONOutputCheck on profiler_get_profile() output - r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/0c9928eb7790
Profiler gtests: Use JSONOutputCheck in GeckoProfiler.Markers gtest - r=canaltinova

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/d6f7e04dd094
https://hg.mozilla.org/mozilla-central/rev/a38ce531d418
https://hg.mozilla.org/mozilla-central/rev/cb22dee08955
https://hg.mozilla.org/mozilla-central/rev/0c9928eb7790