Bug 1670183 - Combine testHas and testGet methods for FOG C++ and JS r=janerik

Differential Revision: https://phabricator.services.mozilla.com/D97032
This commit is contained in:
Chris H-C 2020-11-20 21:39:29 +00:00
Родитель 29c6bf9e69
Коммит 05cf7d0d5d
14 изменённых файлов: 110 добавлений и 282 удалений

Просмотреть файл

@ -23,16 +23,15 @@ GleanCounter::Add(uint32_t amount, JSContext* cx) {
}
NS_IMETHODIMP
GleanCounter::TestHasValue(const nsACString& aStorageName, JSContext* cx,
bool* result) {
*result = this->mCounter.TestHasValue(PromiseFlatCString(aStorageName).get());
return NS_OK;
}
NS_IMETHODIMP
GleanCounter::TestGetValue(const nsACString& aStorageName, JSContext* cx,
int32_t* result) {
*result = this->mCounter.TestGetValue(PromiseFlatCString(aStorageName).get());
GleanCounter::TestGetValue(const nsACString& aStorageName,
JS::MutableHandleValue aResult) {
auto result =
this->mCounter.TestGetValue(PromiseFlatCString(aStorageName).get());
if (result.isNothing()) {
aResult.set(JS::UndefinedValue());
} else {
aResult.set(JS::Int32Value(result.value()));
}
return NS_OK;
}

Просмотреть файл

@ -30,24 +30,6 @@ class CounterMetric {
*/
void Add(int32_t amount = 1) const { fog_counter_add(mId, amount); }
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
bool TestHasValue(const char* aStorageName) const {
return fog_counter_test_has_value(mId, aStorageName) != 0;
}
/**
* **Test-only API**
*
@ -60,10 +42,13 @@ class CounterMetric {
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or Nothing() if there is no value.
*/
int32_t TestGetValue(const char* aStorageName) const {
return fog_counter_test_get_value(mId, aStorageName);
Maybe<int32_t> TestGetValue(const char* aStorageName) const {
if (!fog_counter_test_has_value(mId, aStorageName)) {
return Nothing();
}
return Some(fog_counter_test_get_value(mId, aStorageName));
}
private:

Просмотреть файл

@ -31,18 +31,16 @@ GleanDatetime::Set(PRTime value, uint8_t optional_argc) {
}
NS_IMETHODIMP
GleanDatetime::TestHasValue(const nsACString& aStorageName, JSContext* cx,
bool* result) {
*result =
this->mDatetime.TestHasValue(PromiseFlatCString(aStorageName).get());
return NS_OK;
}
NS_IMETHODIMP
GleanDatetime::TestGetValue(const nsACString& aStorageName, JSContext* cx,
nsACString& result) {
result.Assign(
this->mDatetime.TestGetValue(PromiseFlatCString(aStorageName).get()));
GleanDatetime::TestGetValue(const nsACString& aStorageName, JSContext* aCx,
JS::MutableHandleValue aResult) {
auto result = mDatetime.TestGetValue(PromiseFlatCString(aStorageName).get());
if (result.isNothing()) {
aResult.set(JS::UndefinedValue());
} else {
const NS_ConvertUTF8toUTF16 str(result.value());
aResult.set(
JS::StringValue(JS_NewUCStringCopyN(aCx, str.Data(), str.Length())));
}
return NS_OK;
}

Просмотреть файл

@ -44,26 +44,8 @@ class DatetimeMetric {
int32_t offset =
exploded.tm_params.tp_gmt_offset + exploded.tm_params.tp_dst_offset;
fog_datetime_set(mId, exploded.tm_year, exploded.tm_month + 1,
exploded.tm_mday, exploded.tm_hour, exploded.tm_min,
exploded.tm_sec, exploded.tm_usec * 1000, offset);
}
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
bool TestHasValue(const char* aStorageName) const {
return fog_datetime_test_has_value(mId, aStorageName) != 0;
exploded.tm_mday, exploded.tm_hour, exploded.tm_min,
exploded.tm_sec, exploded.tm_usec * 1000, offset);
}
/**
@ -78,12 +60,15 @@ class DatetimeMetric {
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or Nothing() if there is no value.
*/
nsCString TestGetValue(const char* aStorageName) const {
Maybe<nsCString> TestGetValue(const char* aStorageName) const {
if (!fog_datetime_test_has_value(mId, aStorageName)) {
return Nothing();
}
nsCString ret;
fog_datetime_test_get_value(mId, aStorageName, ret);
return ret;
return Some(ret);
}
private:

Просмотреть файл

@ -22,18 +22,18 @@ GleanString::Set(const nsACString& value, JSContext* cx) {
return NS_OK;
}
NS_IMETHODIMP
GleanString::TestHasValue(const nsACString& aStorageName, JSContext* cx,
bool* result) {
*result = this->mString.TestHasValue(PromiseFlatCString(aStorageName).get());
return NS_OK;
}
NS_IMETHODIMP
GleanString::TestGetValue(const nsACString& aStorageName, JSContext* cx,
nsACString& result) {
result.Assign(
this->mString.TestGetValue(PromiseFlatCString(aStorageName).get()));
JS::MutableHandleValue aResult) {
auto result =
this->mString.TestGetValue(PromiseFlatCString(aStorageName).get());
if (result.isNothing()) {
aResult.set(JS::UndefinedValue());
} else {
const NS_ConvertUTF8toUTF16 str(result.value());
aResult.set(
JS::StringValue(JS_NewUCStringCopyN(cx, str.Data(), str.Length())));
}
return NS_OK;
}

Просмотреть файл

@ -35,24 +35,6 @@ class StringMetric {
*/
void Set(const nsACString& value) const { fog_string_set(mId, value); }
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
bool TestHasValue(const char* aStorageName) const {
return fog_string_test_has_value(mId, aStorageName) != 0;
}
/**
* **Test-only API**
*
@ -65,12 +47,15 @@ class StringMetric {
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or Nothing() if there is no value.
*/
nsCString TestGetValue(const char* aStorageName) const {
Maybe<nsCString> TestGetValue(const char* aStorageName) const {
if (!fog_string_test_has_value(mId, aStorageName)) {
return Nothing();
}
nsCString ret;
fog_string_test_get_value(mId, aStorageName, ret);
return ret;
return Some(ret);
}
private:

Просмотреть файл

@ -29,18 +29,15 @@ GleanTimespan::Stop(JSContext* cx) {
}
NS_IMETHODIMP
GleanTimespan::TestHasValue(const nsACString& aStorageName, JSContext* cx,
bool* result) {
*result =
this->mTimespan.TestHasValue(PromiseFlatCString(aStorageName).get());
return NS_OK;
}
NS_IMETHODIMP
GleanTimespan::TestGetValue(const nsACString& aStorageName, JSContext* cx,
int64_t* result) {
*result =
GleanTimespan::TestGetValue(const nsACString& aStorageName,
JS::MutableHandleValue aResult) {
auto result =
this->mTimespan.TestGetValue(PromiseFlatCString(aStorageName).get());
if (result.isNothing()) {
aResult.set(JS::UndefinedValue());
} else {
aResult.set(JS::DoubleValue(result.value()));
}
return NS_OK;
}

Просмотреть файл

@ -43,24 +43,6 @@ class TimespanMetric {
*/
void Stop() const { fog_timespan_stop(mId); }
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
bool TestHasValue(const char* aStorageName) const {
return fog_timespan_test_has_value(mId, aStorageName) != 0;
}
/**
* **Test-only API**
*
@ -73,10 +55,13 @@ class TimespanMetric {
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or Nothing() if there is no value.
*/
int64_t TestGetValue(const char* aStorageName) const {
return fog_timespan_test_get_value(mId, aStorageName);
Maybe<int64_t> TestGetValue(const char* aStorageName) const {
if (!fog_timespan_test_has_value(mId, aStorageName)) {
return Nothing();
}
return Some(fog_timespan_test_get_value(mId, aStorageName));
}
private:

Просмотреть файл

@ -28,18 +28,18 @@ GleanUuid::GenerateAndSet(JSContext* cx) {
return NS_OK;
}
NS_IMETHODIMP
GleanUuid::TestHasValue(const nsACString& aStorageName, JSContext* cx,
bool* result) {
*result = this->mUuid.TestHasValue(PromiseFlatCString(aStorageName).get());
return NS_OK;
}
NS_IMETHODIMP
GleanUuid::TestGetValue(const nsACString& aStorageName, JSContext* cx,
nsACString& result) {
result.Assign(
this->mUuid.TestGetValue(PromiseFlatCString(aStorageName).get()));
JS::MutableHandleValue aResult) {
auto result =
this->mUuid.TestGetValue(PromiseFlatCString(aStorageName).get());
if (result.isNothing()) {
aResult.set(JS::UndefinedValue());
} else {
const NS_ConvertUTF8toUTF16 str(result.value());
aResult.set(
JS::StringValue(JS_NewUCStringCopyN(cx, str.Data(), str.Length())));
}
return NS_OK;
}

Просмотреть файл

@ -38,24 +38,6 @@ class UuidMetric {
*/
void GenerateAndSet() const { fog_uuid_generate_and_set(mId); }
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
bool TestHasValue(const char* aStorageName) const {
return fog_uuid_test_has_value(mId, aStorageName) != 0;
}
/**
* **Test-only API**
*
@ -69,12 +51,15 @@ class UuidMetric {
* Parent process only. Panics in child processes.
* Panics if there is no value to get.
*
* @return value of the stored metric.
* @return value of the stored metric, or Nothing() if there is no value.
*/
nsCString TestGetValue(const char* aStorageName) const {
Maybe<nsCString> TestGetValue(const char* aStorageName) const {
if (!fog_uuid_test_has_value(mId, aStorageName)) {
return Nothing();
}
nsCString ret;
fog_uuid_test_get_value(mId, aStorageName, ret);
return ret;
return Some(ret);
}
private:

Просмотреть файл

@ -89,6 +89,9 @@ Each metric type has six pieces you'll need to cover:
[`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
Its methods should be named the same as the ones in the Rust API,
transformed to `CamelCase`. They should all be public.
Multiplex the FFI's `test_have` and `test_get` functions into a single
`TestGetValue` function that returns a
`mozilla::Maybe` wrapping the C++ type that best fits the metric type.
3. **IDL** - Duplicate the public API (including its docs) to
[`xpcom/nsIGleanMetrics.idl`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/xpcom/nsIGleanMetrics.idl)
with the name `nsIGleanX` (e.g. `nsIGleanCounter`).
@ -98,6 +101,7 @@ Each metric type has six pieces you'll need to cover:
[GUID](https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Generating_GUIDs)
because this is XPCOM,
but you'll only need the canonical form since we're only exposing to JS.
The `testGetValue` method will return a `jsval`.
4. **JS Impl** - Add an `nsIGleanX`-deriving, `XMetric`-owning type called `GleanX`
(e.g. `GleanCounter`) in the same header and `.cpp` as `XMetric` in
[`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
@ -109,7 +113,9 @@ Each metric type has six pieces you'll need to cover:
`CamelCase` and need macros like `NS_IMETHODIMP`.
Delegate operations to the owned `XMetric`, returning `NS_OK`
no matter what in non-test methods.
(Test-only methods can return `NS_ERROR` codes on failures).
Test-only methods can return `NS_ERROR` codes on failures,
but mostly return `NS_OK` and use `undefined` in the
`JS::MutableHandleValue` result to signal no value.
5. **Tests** - Two languages means two test suites.
Add a never-expiring test-only metric of your type to
[`test_metrics.yaml`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/test_metrics.yaml).

Просмотреть файл

@ -56,8 +56,9 @@ TEST(FOG, TestCppCounterWorks)
{
mozilla::glean::test_only::bad_code.Add(42);
ASSERT_TRUE(mozilla::glean::test_only::bad_code.TestHasValue("test-ping"));
ASSERT_EQ(42, mozilla::glean::test_only::bad_code.TestGetValue("test-ping"));
ASSERT_EQ(
42,
mozilla::glean::test_only::bad_code.TestGetValue("test-ping").value());
}
TEST(FOG, TestCppStringWorks)
@ -65,11 +66,10 @@ TEST(FOG, TestCppStringWorks)
auto kValue = "cheez!"_ns;
mozilla::glean::test_only::cheesy_string.Set(kValue);
ASSERT_TRUE(
mozilla::glean::test_only::cheesy_string.TestHasValue("test-ping"));
ASSERT_STREQ(
kValue.get(),
mozilla::glean::test_only::cheesy_string.TestGetValue("test-ping").get());
ASSERT_STREQ(kValue.get(), mozilla::glean::test_only::cheesy_string
.TestGetValue("test-ping")
.value()
.get());
}
TEST(FOG, TestCppTimespanWorks)
@ -79,34 +79,29 @@ TEST(FOG, TestCppTimespanWorks)
mozilla::glean::test_only::can_we_time_it.Stop();
ASSERT_TRUE(
mozilla::glean::test_only::can_we_time_it.TestHasValue("test-ping"));
ASSERT_TRUE(
mozilla::glean::test_only::can_we_time_it.TestGetValue("test-ping") > 0);
mozilla::glean::test_only::can_we_time_it.TestGetValue("test-ping")
.value() > 0);
}
TEST(FOG, TestCppUuidWorks)
{
nsCString kTestUuid("decafdec-afde-cafd-ecaf-decafdecafde");
test_only::what_id_it.Set(kTestUuid);
ASSERT_TRUE(test_only::what_id_it.TestHasValue("test-ping"));
ASSERT_STREQ(kTestUuid.get(),
test_only::what_id_it.TestGetValue("test-ping").get());
test_only::what_id_it.TestGetValue("test-ping").value().get());
test_only::what_id_it.GenerateAndSet();
// Since we generate v4 UUIDs, and the first character of the third group
// isn't 4, this won't ever collide with kTestUuid.
ASSERT_STRNE(kTestUuid.get(),
test_only::what_id_it.TestGetValue("test-ping").get());
test_only::what_id_it.TestGetValue("test-ping").value().get());
}
TEST(FOG, TestCppDatetimeWorks)
{
PRExplodedTime date = {0, 35, 10, 12, 6, 10, 2020, 0, 0, {5 * 60 * 60, 0}};
mozilla::glean::test_only::what_a_date.Set(&date);
ASSERT_TRUE(mozilla::glean::test_only::what_a_date.TestHasValue("test-ping"));
test_only::what_a_date.Set(&date);
nsCString received;
received.Assign(
mozilla::glean::test_only::what_a_date.TestGetValue("test-ping"));
ASSERT_STREQ(received.get(), "2020-11-06T12:10:35+05:00");
auto received = test_only::what_a_date.TestGetValue("test-ping");
ASSERT_STREQ(received.value().get(), "2020-11-06T12:10:35+05:00");
}

Просмотреть файл

@ -16,23 +16,6 @@ interface nsIGleanDatetime : nsISupports
[optional_argc]
void set([optional] in PRTime value);
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
[implicit_jscontext]
bool testHasValue(in ACString aStorageName);
/**
* **Test-only API**
*
@ -45,10 +28,10 @@ interface nsIGleanDatetime : nsISupports
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or undefined if there is no value.
*/
[implicit_jscontext]
ACString testGetValue(in ACString aStorageName);
jsval testGetValue(in ACString aStorageName);
};
[scriptable, uuid(05b89d2a-d57c-11ea-82da-3f63399a6f5a)]
@ -62,23 +45,6 @@ interface nsIGleanCounter : nsISupports
[implicit_jscontext]
void add(in uint32_t amount);
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
[implicit_jscontext]
bool testHasValue(in ACString aStorageName);
/**
* **Test-only API**
*
@ -91,10 +57,9 @@ interface nsIGleanCounter : nsISupports
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or undefined if there is no value.
*/
[implicit_jscontext]
long testGetValue(in ACString aStorageName);
jsval testGetValue(in ACString aStorageName);
};
[scriptable, uuid(d84a3555-46f1-48c1-9122-e8e88b069d2b)]
@ -108,23 +73,6 @@ interface nsIGleanString : nsISupports
[implicit_jscontext]
void set(in ACString value);
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
[implicit_jscontext]
bool testHasValue(in ACString aStorageName);
/**
* **Test-only API**
*
@ -137,10 +85,10 @@ interface nsIGleanString : nsISupports
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or undefined if there is no value.
*/
[implicit_jscontext]
ACString testGetValue(in ACString aStorageName);
jsval testGetValue(in ACString aStorageName);
};
[scriptable, uuid(2586530c-030f-11eb-93cb-cbf30d25225a)]
@ -167,23 +115,6 @@ interface nsIGleanTimespan : nsISupports
[implicit_jscontext]
void stop();
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
[implicit_jscontext]
bool testHasValue(in ACString aStorageName);
/**
* **Test-only API**
*
@ -196,10 +127,9 @@ interface nsIGleanTimespan : nsISupports
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or undefined if there is no value.
*/
[implicit_jscontext]
long long testGetValue(in ACString aStorageName);
jsval testGetValue(in ACString aStorageName);
};
[scriptable, uuid(395700e7-06f6-46be-adcc-ea58977fda6d)]
@ -219,23 +149,6 @@ interface nsIGleanUuid : nsISupports
[implicit_jscontext]
void generateAndSet();
/**
* **Test-only API**
*
* Tests whether a value is stored for the metric.
*
* This function will attempt to await the last parent-process task (if any)
* writing to the the metric's storage engine before returning a value.
* This function will not wait for data from child processes.
*
* Parent process only. Panics in child processes.
*
* @param aStorageName the name of the ping to retrieve the metric for.
* @return true if metric value exists, otherwise false
*/
[implicit_jscontext]
bool testHasValue(in ACString aStorageName);
/**
* **Test-only API**
*
@ -248,8 +161,8 @@ interface nsIGleanUuid : nsISupports
* This doesn't clear the stored value.
* Parent process only. Panics in child processes.
*
* @return value of the stored metric.
* @return value of the stored metric, or undefined if there is no value.
*/
[implicit_jscontext]
ACString testGetValue(in ACString aStorageName);
jsval testGetValue(in ACString aStorageName);
};

Просмотреть файл

@ -28,7 +28,6 @@ add_task(function test_setup() {
add_task(function test_fog_counter_works() {
Glean.test_only.bad_code.add(31);
Assert.ok(Glean.test_only.bad_code.testHasValue("test-ping"));
Assert.equal(31, Glean.test_only.bad_code.testGetValue("test-ping"));
});
@ -36,7 +35,6 @@ add_task(async function test_fog_string_works() {
const value = "a cheesy string!";
Glean.test_only.cheesy_string.set(value);
Assert.ok(Glean.test_only.cheesy_string.testHasValue("test-ping"));
Assert.equal(value, Glean.test_only.cheesy_string.testGetValue("test-ping"));
});
@ -47,14 +45,12 @@ add_task(async function test_fog_timespan_works() {
await sleep(10);
Glean.test_only.can_we_time_it.stop();
Assert.ok(Glean.test_only.can_we_time_it.testHasValue("test-ping"));
Assert.ok(Glean.test_only.can_we_time_it.testGetValue("test-ping") > 0);
});
add_task(async function test_fog_uuid_works() {
const kTestUuid = "decafdec-afde-cafd-ecaf-decafdecafde";
Glean.test_only.what_id_it.set(kTestUuid);
Assert.ok(Glean.test_only.what_id_it.testHasValue("test-ping"));
Assert.equal(kTestUuid, Glean.test_only.what_id_it.testGetValue("test-ping"));
Glean.test_only.what_id_it.generateAndSet();
@ -70,7 +66,6 @@ add_task(function test_fog_datetime_works() {
const value = new Date("2020-06-11T12:00:00");
Glean.test_only.what_a_date.set(value.getTime() * 1000);
Assert.ok(Glean.test_only.what_a_date.testHasValue("test-ping"));
const received = Glean.test_only.what_a_date.testGetValue("test-ping");
Assert.ok(received.startsWith("2020-06-11T12:00:00"));