From e11473aa2e41a59fe1f8569ddc3fd98be50b92c2 Mon Sep 17 00:00:00 2001 From: Chris H-C Date: Mon, 18 Jul 2022 14:24:33 +0000 Subject: [PATCH] Bug 1756057 - For GTest create FOGFixture for ensuring FOG is properly set up for your test case r=janerik Depends on D147449 Differential Revision: https://phabricator.services.mozilla.com/D147450 --- .../glean/docs/user/instrumentation_tests.md | 63 ++++++++++++++----- .../components/glean/tests/gtest/FOGFixture.h | 23 +++++++ .../components/glean/tests/gtest/TestFog.cpp | 61 +++++++----------- 3 files changed, 90 insertions(+), 57 deletions(-) create mode 100644 toolkit/components/glean/tests/gtest/FOGFixture.h diff --git a/toolkit/components/glean/docs/user/instrumentation_tests.md b/toolkit/components/glean/docs/user/instrumentation_tests.md index fc1ec1f0f203..ed28c6d4e873 100644 --- a/toolkit/components/glean/docs/user/instrumentation_tests.md +++ b/toolkit/components/glean/docs/user/instrumentation_tests.md @@ -33,8 +33,9 @@ you're going to want to write some automated tests. * You may see values from previous tests persist across tests because the profile directory was shared between test cases. * You can reset Glean before your test by calling - `Services.fog.testResetFOG()` (in JS), or - `mozilla::glean::TestResetFOG()` (in C++). + `Services.fog.testResetFOG()` (in JS). + * You shouldn't have to do this in C++ or Rust since there you should use the + `FOGFixture` test fixture. * If your metric is based on timing (`timespan`, `timing_distribution`), do not expect to be able to assert the correct timing value. Glean does a lot of timing for you deep in the SDK, so unless you mock the system's monotonic clock, @@ -44,6 +45,9 @@ you're going to want to write some automated tests. but beware of rounding. * Errors in instrumentation APIs do not panic, throw, or crash. But Glean remembers that the errors happened. + * Test APIs, on the other hand, are permitted + (some may say "encouraged") + to panic, throw, or crash on bad behaviour. * If you call a test API and it panics, throws, or crashes, that means your instrumentation did something wrong. Check your test logs for details about what went awry. @@ -157,18 +161,24 @@ In this case there's a slight addition to the Usual Test Format: ## GTests/Google Tests -Unfortunately this is a pain to test at the moment. -gtests run with a single temporary profile, but FOG's own gtests use it, -polluting it from test to test. +Please make use of the `FOGFixture` fixture when writing your tests, like: -In this case there's a slight addition to the Usual Test Format: -1) _Reset FOG with `mozilla::glean::impl::fog_test_reset(""_ns, ""_ns);`_ -2) Assert no value in the metric -3) Express behaviour -4) Assert correct value in the metric. +```cpp +TEST_F(FOGFixture, MyTestCase) { + // 1) Assert no value + ASSERT_EQ(mozilla::Nothing(), + my_metric_category::my_metric_name.TestGetValue()); -Work to improve this is tracked in -[bug 1756057](https://bugzilla.mozilla.org/show_bug.cgi?id=1756057). + // 2) Express behaviour + // ...... + + // 3) Assert correct value + ASSERT_EQ(kValue, + my_metric_category::my_metric_name.TestGetValue().unwrap().ref()); +} +``` + +The fixture will take care of ensuring storage is reset between tests. ## Rust `rusttests` @@ -182,11 +192,30 @@ which means we need to use the [GTest + FFI approach](/testing-rust-code/index.html#gtests) where GTest is the runner and Rust is just the language the test is written in. -This means you follow the GTests/Google Tests variant on the Usual Test Format: -1) _Reset FOG with `mozilla::glean::impl::fog_test_reset(""_ns, ""_ns);`_ -2) Assert no value in the metric -3) Express behaviour -4) Assert correct value in the metric. +This means your test will look like a GTest like this: + +```cpp +extern "C" void Rust_MyRustTest(); +TEST_F(FOGFixture, MyRustTest) { Rust_MyRustTest(); } +``` + +Plus a Rust test like this: + +```rust +#[no_mangle] +pub extern "C" fn Rust_MyRustTest() { + // 1) Assert no value + assert_eq!(None, + fog::metrics::my_metric_category::my_metric_name.test_get_value(None)); + + // 2) Express behaviour + // ...... + + // 3) Assert correct value + assert_eq!(Some(value), + fog::metrics::my_metric_category::my_metric_name.test_get_value(None)); +} +``` [glean-metrics-apis]: https://mozilla.github.io/glean/book/reference/metrics/index.html [metrics-xpcshell-test]: https://searchfox.org/mozilla-central/rev/66e59131c1c76fe486424dc37f0a8a399ca874d4/toolkit/mozapps/update/tests/unit_background_update/test_backgroundupdate_glean.js#28 diff --git a/toolkit/components/glean/tests/gtest/FOGFixture.h b/toolkit/components/glean/tests/gtest/FOGFixture.h new file mode 100644 index 000000000000..da3c6fc12318 --- /dev/null +++ b/toolkit/components/glean/tests/gtest/FOGFixture.h @@ -0,0 +1,23 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +#ifndef FOGFixture_h_ +#define FOGFixture_h_ + +#include "gtest/gtest.h" +#include "mozilla/glean/fog_ffi_generated.h" +#include "nsString.h" + +using namespace mozilla::glean::impl; + +class FOGFixture : public ::testing::Test { + protected: + FOGFixture() = default; + virtual void SetUp() { + nsCString empty; + ASSERT_EQ(NS_OK, fog_test_reset(&empty, &empty)); + } +}; + +#endif // FOGFixture_h_ diff --git a/toolkit/components/glean/tests/gtest/TestFog.cpp b/toolkit/components/glean/tests/gtest/TestFog.cpp index 09b5c983a192..65b3c5fffc02 100644 --- a/toolkit/components/glean/tests/gtest/TestFog.cpp +++ b/toolkit/components/glean/tests/gtest/TestFog.cpp @@ -2,6 +2,7 @@ * 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/. */ +#include "FOGFixture.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "mozilla/glean/GleanMetrics.h" @@ -32,8 +33,7 @@ void GTest_FOG_ExpectFailure(const char* aMessage) { } } -TEST(FOG, BuiltinPingsRegistered) -{ +TEST_F(FOGFixture, BuiltinPingsRegistered) { Preferences::SetInt("telemetry.fog.test.localhost_port", -1); nsAutoCString metricsPingName("metrics"); nsAutoCString baselinePingName("baseline"); @@ -43,8 +43,7 @@ TEST(FOG, BuiltinPingsRegistered) ASSERT_EQ(NS_OK, fog_submit_ping(&eventsPingName)); } -TEST(FOG, TestCppCounterWorks) -{ +TEST_F(FOGFixture, TestCppCounterWorks) { mozilla::glean::test_only::bad_code.Add(42); ASSERT_EQ(42, mozilla::glean::test_only::bad_code.TestGetValue("test-ping"_ns) @@ -54,8 +53,7 @@ TEST(FOG, TestCppCounterWorks) ASSERT_EQ(42, test_only::bad_code.TestGetValue().unwrap().value()); } -TEST(FOG, TestCppStringWorks) -{ +TEST_F(FOGFixture, TestCppStringWorks) { auto kValue = "cheez!"_ns; mozilla::glean::test_only::cheesy_string.Set(kValue); @@ -66,8 +64,7 @@ TEST(FOG, TestCppStringWorks) .get()); } -TEST(FOG, TestCppTimespanWorks) -{ +TEST_F(FOGFixture, TestCppTimespanWorks) { mozilla::glean::test_only::can_we_time_it.Start(); PR_Sleep(PR_MillisecondsToInterval(10)); mozilla::glean::test_only::can_we_time_it.Stop(); @@ -78,8 +75,7 @@ TEST(FOG, TestCppTimespanWorks) .value() > 0); } -TEST(FOG, TestCppUuidWorks) -{ +TEST_F(FOGFixture, TestCppUuidWorks) { nsCString kTestUuid("decafdec-afde-cafd-ecaf-decafdecafde"); test_only::what_id_it.Set(kTestUuid); ASSERT_STREQ(kTestUuid.get(), @@ -98,8 +94,7 @@ TEST(FOG, TestCppUuidWorks) .get()); } -TEST(FOG, TestCppBooleanWorks) -{ +TEST_F(FOGFixture, TestCppBooleanWorks) { mozilla::glean::test_only::can_we_flag_it.Set(false); ASSERT_EQ(false, mozilla::glean::test_only::can_we_flag_it @@ -113,8 +108,7 @@ MATCHER_P(BitEq, x, "bit equal") { return std::memcmp(&arg, &x, sizeof(x)) == 0; } -TEST(FOG, TestCppDatetimeWorks) -{ +TEST_F(FOGFixture, TestCppDatetimeWorks) { PRExplodedTime date{0, 35, 10, 12, 6, 10, 2020, 0, 0, {5 * 60 * 60, 0}}; test_only::what_a_date.Set(&date); @@ -128,8 +122,7 @@ using mozilla::Tuple; using mozilla::glean::test_only_ipc::AnEventExtra; using mozilla::glean::test_only_ipc::EventWithExtraExtra; -TEST(FOG, TestCppEventWorks) -{ +TEST_F(FOGFixture, TestCppEventWorks) { test_only_ipc::no_extra_event.Record(); ASSERT_TRUE(test_only_ipc::no_extra_event.TestGetValue("store1"_ns) .unwrap() @@ -149,8 +142,7 @@ TEST(FOG, TestCppEventWorks) ASSERT_STREQ("can set extras", mozilla::Get<1>(events[0].mExtra[0]).get()); } -TEST(FOG, TestCppEventsWithDifferentExtraTypes) -{ +TEST_F(FOGFixture, TestCppEventsWithDifferentExtraTypes) { EventWithExtraExtra extra = {.extra1 = Some("can set extras"_ns), .extra2 = Some(37), .extra3LongerName = Some(false)}; @@ -181,8 +173,7 @@ TEST(FOG, TestCppEventsWithDifferentExtraTypes) } } -TEST(FOG, TestCppMemoryDistWorks) -{ +TEST_F(FOGFixture, TestCppMemoryDistWorks) { test_only::do_you_remember.Accumulate(7); test_only::do_you_remember.Accumulate(17); @@ -200,8 +191,7 @@ TEST(FOG, TestCppMemoryDistWorks) } } -TEST(FOG, TestCppCustomDistWorks) -{ +TEST_F(FOGFixture, TestCppCustomDistWorks) { test_only_ipc::a_custom_dist.AccumulateSamples({7, 268435458}); DistributionData data = @@ -216,8 +206,7 @@ TEST(FOG, TestCppCustomDistWorks) } } -TEST(FOG, TestCppPings) -{ +TEST_F(FOGFixture, TestCppPings) { test_only::one_ping_one_bool.Set(false); const auto& ping = mozilla::glean_pings::OnePingOnly; bool submitted = false; @@ -231,8 +220,7 @@ TEST(FOG, TestCppPings) << "Must have actually called the lambda."; } -TEST(FOG, TestCppStringLists) -{ +TEST_F(FOGFixture, TestCppStringLists) { auto kValue = "cheez!"_ns; auto kValue2 = "cheezier!"_ns; auto kValue3 = "cheeziest."_ns; @@ -254,8 +242,7 @@ TEST(FOG, TestCppStringLists) ASSERT_STREQ(kValue3.get(), val[2].get()); } -TEST(FOG, TestCppTimingDistWorks) -{ +TEST_F(FOGFixture, TestCppTimingDistWorks) { auto id1 = test_only::what_time_is_it.Start(); auto id2 = test_only::what_time_is_it.Start(); PR_Sleep(PR_MillisecondsToInterval(5)); @@ -285,8 +272,7 @@ TEST(FOG, TestCppTimingDistWorks) ASSERT_EQ(sampleCount, (uint64_t)2); } -TEST(FOG, TestLabeledBooleanWorks) -{ +TEST_F(FOGFixture, TestLabeledBooleanWorks) { ASSERT_EQ(mozilla::Nothing(), test_only::mabels_like_balloons.Get("hot_air"_ns) .TestGetValue() @@ -303,8 +289,7 @@ TEST(FOG, TestLabeledBooleanWorks) .ref()); } -TEST(FOG, TestLabeledCounterWorks) -{ +TEST_F(FOGFixture, TestLabeledCounterWorks) { ASSERT_EQ(mozilla::Nothing(), test_only::mabels_kitchen_counters.Get("marble"_ns) .TestGetValue() @@ -321,8 +306,7 @@ TEST(FOG, TestLabeledCounterWorks) .ref()); } -TEST(FOG, TestLabeledStringWorks) -{ +TEST_F(FOGFixture, TestLabeledStringWorks) { ASSERT_EQ(mozilla::Nothing(), test_only::mabels_balloon_strings.Get("twine"_ns) .TestGetValue() @@ -344,8 +328,7 @@ TEST(FOG, TestLabeledStringWorks) .get()); } -TEST(FOG, TestCppQuantityWorks) -{ +TEST_F(FOGFixture, TestCppQuantityWorks) { // This joke only works in base 13. const uint32_t kValue = 6 * 9; mozilla::glean::test_only::meaning_of_life.Set(kValue); @@ -355,8 +338,7 @@ TEST(FOG, TestCppQuantityWorks) .value()); } -TEST(FOG, TestCppRateWorks) -{ +TEST_F(FOGFixture, TestCppRateWorks) { // 1) Standard rate with internal denominator const int32_t kNum = 22; const int32_t kDen = 7; // because I like pi, even just approximately. @@ -378,8 +360,7 @@ TEST(FOG, TestCppRateWorks) test_only_ipc::an_external_denominator.TestGetValue().unwrap().extract()); } -TEST(FOG, TestCppUrlWorks) -{ +TEST_F(FOGFixture, TestCppUrlWorks) { auto kValue = "https://example.com/fog/gtest"_ns; mozilla::glean::test_only_ipc::a_url.Set(kValue);