From b50e81a13f1f207f67250a68c455ec75e93558b9 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Mon, 3 May 2021 20:26:07 +0200 Subject: [PATCH 1/4] Rust: Don't return a result from `submit_ping` The `Result` value returned previously was not expressing what's really happening anymore. It was impossible to encounter an `Err` variant after recent changes. Technically this is a breaking change for glean-core, but that library is not consumed by any outsiders. --- CHANGELOG.md | 3 ++ glean-core/ffi/src/lib.rs | 4 +-- glean-core/rlb/src/lib.rs | 11 +++----- glean-core/src/event_database/mod.rs | 19 ++----------- glean-core/src/lib.rs | 41 ++++++++++++++++------------ glean-core/src/lib_unit_tests.rs | 7 ++--- glean-core/src/metrics/ping.rs | 3 +- glean-core/src/upload/directory.rs | 12 +++----- glean-core/src/upload/mod.rs | 30 +++++++++----------- glean-core/tests/ping.rs | 16 +++++------ glean-core/tests/ping_maker.rs | 10 +++---- 11 files changed, 67 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b81f488583..62e9aaaf0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ [Full changelog](https://github.com/mozilla/glean/compare/v37.0.0...main) +* Rust + * Don't return a result from `submit_ping`. The boolean return value indicates whether a ping was submitted ([#1613](https://github.com/mozilla/glean/pull/1613)) + # v37.0.0 (2021-04-30) [Full changelog](https://github.com/mozilla/glean/compare/v36.0.1...v37.0.0) diff --git a/glean-core/ffi/src/lib.rs b/glean-core/ffi/src/lib.rs index d6f07fe2af..77737eddf7 100644 --- a/glean-core/ffi/src/lib.rs +++ b/glean-core/ffi/src/lib.rs @@ -301,9 +301,7 @@ pub extern "C" fn glean_set_upload_enabled(flag: u8) { #[no_mangle] pub extern "C" fn glean_submit_ping_by_name(ping_name: FfiStr, reason: FfiStr) -> u8 { with_glean(|glean| { - Ok(glean - .submit_ping_by_name(&ping_name.to_string_fallible()?, reason.as_opt_str()) - .unwrap_or(false)) + Ok(glean.submit_ping_by_name(&ping_name.to_string_fallible()?, reason.as_opt_str())) }) } diff --git a/glean-core/rlb/src/lib.rs b/glean-core/rlb/src/lib.rs index 438acd6853..671d383f48 100644 --- a/glean-core/rlb/src/lib.rs +++ b/glean-core/rlb/src/lib.rs @@ -313,10 +313,7 @@ pub fn initialize(cfg: Configuration, client_info: ClientInfoMetrics) { // write lock on the `glean` object. // Note that unwrapping below is safe: the function will return an // `Ok` value for a known ping. - if glean - .submit_ping_by_name("baseline", Some("dirty_startup")) - .unwrap() - { + if glean.submit_ping_by_name("baseline", Some("dirty_startup")) { state.upload_manager.trigger_upload(); } } @@ -553,13 +550,13 @@ pub(crate) fn submit_ping_by_name_sync(ping: &str, reason: Option<&str>) { // This won't actually return from `submit_ping_by_name`, but // returning `false` here skips spinning up the uploader below, // which is basically the same. - return Some(false); + return false; } - glean.submit_ping_by_name(&ping, reason.as_deref()).ok() + glean.submit_ping_by_name(&ping, reason.as_deref()) }); - if let Some(true) = submitted_ping { + if submitted_ping { let state = global_state().lock().unwrap(); state.upload_manager.trigger_upload(); } diff --git a/glean-core/src/event_database/mod.rs b/glean-core/src/event_database/mod.rs index 9b14d748cc..245f533130 100644 --- a/glean-core/src/event_database/mod.rs +++ b/glean-core/src/event_database/mod.rs @@ -161,15 +161,7 @@ impl EventDatabase { let mut ping_sent = false; for store_name in store_names { - if let Err(err) = glean.submit_ping_by_name(&store_name, Some("startup")) { - log::warn!( - "Error flushing existing events to the '{}' ping: {}", - store_name, - err - ); - } else { - ping_sent = true; - } + ping_sent |= glean.submit_ping_by_name(&store_name, Some("startup")); } ping_sent @@ -225,14 +217,7 @@ impl EventDatabase { // If any of the event stores reached maximum size, submit the pings // containing those events immediately. for store_name in stores_to_submit { - if let Err(err) = glean.submit_ping_by_name(store_name, Some("max_capacity")) { - log::warn!( - "Got more than {} events, but could not persist {} ping: {}", - glean.get_max_events(), - store_name, - err - ); - } + glean.submit_ping_by_name(store_name, Some("max_capacity")); } } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index fb40c1de63..b22b782fe6 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -161,7 +161,7 @@ pub struct Configuration { /// /// call_counter.add(&glean, 1); /// -/// glean.submit_ping(&ping, None).unwrap(); +/// glean.submit_ping(&ping, None); /// ``` /// /// ## Note @@ -454,8 +454,8 @@ impl Glean { } else { Some("set_upload_enabled") }; - if let Err(err) = self.internal_pings.deletion_request.submit(self, reason) { - log::error!("Failed to submit deletion-request ping on optout: {}", err); + if !self.internal_pings.deletion_request.submit(self, reason) { + log::error!("Failed to submit deletion-request ping on optout."); } self.clear_metrics(); self.upload_enabled = false; @@ -626,10 +626,10 @@ impl Glean { /// # Returns /// /// Whether the ping was succesfully assembled and queued. - pub fn submit_ping(&self, ping: &PingType, reason: Option<&str>) -> Result { + pub fn submit_ping(&self, ping: &PingType, reason: Option<&str>) -> bool { if !self.is_upload_enabled() { log::info!("Glean disabled: not submitting any pings."); - return Ok(false); + return false; } let ping_maker = PingMaker::new(); @@ -641,7 +641,7 @@ impl Glean { "No content for ping '{}', therefore no ping queued.", ping.name ); - Ok(false) + false } Some(ping) => { // This metric is recorded *after* the ping is collected (since @@ -656,7 +656,12 @@ impl Glean { if let Err(e) = ping_maker.store_ping(&self.get_data_path(), &ping) { log::warn!("IO error while writing ping to file: {}. Enqueuing upload of what we have in memory.", e); self.additional_metrics.io_errors.add(self, 1); - let content = ::serde_json::to_string(&ping.content)?; + // `serde_json::to_string` only fails if serialization of the content + // fails or it contains maps with non-string keys. + // However `ping.content` is already a `JsonValue`, + // so both scenarios should be impossible. + let content = + ::serde_json::to_string(&ping.content).expect("ping serialization failed"); self.upload_manager.enqueue_ping( self, ping.doc_id, @@ -664,8 +669,7 @@ impl Glean { &content, Some(ping.headers), ); - // Not actually 100% 'Ok'. bug 1704606 - return Ok(true); + return true; } self.upload_manager.enqueue_ping_from_file(self, &doc_id); @@ -674,7 +678,8 @@ impl Glean { "The ping '{}' was submitted and will be sent as soon as possible", ping.name ); - Ok(true) + + true } } } @@ -699,11 +704,11 @@ impl Glean { /// # Errors /// /// If collecting or writing the ping to disk failed. - pub fn submit_ping_by_name(&self, ping_name: &str, reason: Option<&str>) -> Result { + pub fn submit_ping_by_name(&self, ping_name: &str, reason: Option<&str>) -> bool { match self.get_ping_by_name(ping_name) { None => { log::error!("Attempted to submit unknown ping '{}'", ping_name); - Ok(false) + false } Some(ping) => self.submit_ping(ping, reason), } @@ -917,8 +922,8 @@ impl Glean { /// This functions generates a baseline ping with reason `active` /// and then sets the dirty bit. pub fn handle_client_active(&mut self) { - if let Err(err) = self.internal_pings.baseline.submit(self, Some("active")) { - log::warn!("Failed to submit baseline ping on active: {}", err); + if !self.internal_pings.baseline.submit(self, Some("active")) { + log::warn!("Failed to submit baseline ping on active"); } self.set_dirty_flag(true); @@ -929,12 +934,12 @@ impl Glean { /// This functions generates a baseline and an events ping with reason /// `inactive` and then clears the dirty bit. pub fn handle_client_inactive(&mut self) { - if let Err(err) = self.internal_pings.baseline.submit(self, Some("inactive")) { - log::warn!("Failed to submit baseline ping on inactive: {}", err); + if !self.internal_pings.baseline.submit(self, Some("inactive")) { + log::warn!("Failed to submit baseline ping on inactive"); } - if let Err(err) = self.internal_pings.events.submit(self, Some("inactive")) { - log::warn!("Failed to submit events ping on inactive: {}", err); + if !self.internal_pings.events.submit(self, Some("inactive")) { + log::warn!("Failed to submit events ping on inactive"); } self.set_dirty_flag(false); diff --git a/glean-core/src/lib_unit_tests.rs b/glean-core/src/lib_unit_tests.rs index 52e4a8bf9d..eddd90660b 100644 --- a/glean-core/src/lib_unit_tests.rs +++ b/glean-core/src/lib_unit_tests.rs @@ -893,9 +893,8 @@ fn records_io_errors() { // Writing the ping file should fail. let submitted = glean.internal_pings.metrics.submit(&glean, None); - // But the return value is still Ok(true) because we enqueue the ping anyway. - assert!(submitted.is_ok()); - assert!(submitted.unwrap()); + // But the return value is still `true` because we enqueue the ping anyway. + assert!(submitted); let metric = &glean.additional_metrics.io_errors; assert_eq!( @@ -909,7 +908,7 @@ fn records_io_errors() { // Now we can submit a ping let submitted = glean.internal_pings.metrics.submit(&glean, None); - assert!(submitted.is_ok()); + assert!(submitted); } #[test] diff --git a/glean-core/src/metrics/ping.rs b/glean-core/src/metrics/ping.rs index fd44b06dec..2aacbcf3a1 100644 --- a/glean-core/src/metrics/ping.rs +++ b/glean-core/src/metrics/ping.rs @@ -2,7 +2,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use crate::error::Result; use crate::Glean; /// Stores information about a ping. @@ -60,7 +59,7 @@ impl PingType { /// # Returns /// /// See [`Glean::submit_ping`](crate::Glean::submit_ping) for details. - pub fn submit(&self, glean: &Glean, reason: Option<&str>) -> Result { + pub fn submit(&self, glean: &Glean, reason: Option<&str>) -> bool { let corrected_reason = match reason { Some(reason) => { if self.reason_codes.contains(&reason.to_string()) { diff --git a/glean-core/src/upload/directory.rs b/glean-core/src/upload/directory.rs index 6b4a58156b..6c7722aabd 100644 --- a/glean-core/src/upload/directory.rs +++ b/glean-core/src/upload/directory.rs @@ -307,7 +307,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit the ping to populate the pending_pings directory - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); let directory_manager = PingDirectoryManager::new(dir.path()); @@ -333,7 +333,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit the ping to populate the pending_pings directory - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); let directory_manager = PingDirectoryManager::new(&dir.path()); @@ -368,7 +368,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit the ping to populate the pending_pings directory - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); let directory_manager = PingDirectoryManager::new(&dir.path()); @@ -399,11 +399,7 @@ mod test { let (glean, dir) = new_glean(None); // Submit a deletion request ping to populate deletion request folder. - glean - .internal_pings - .deletion_request - .submit(&glean, None) - .unwrap(); + glean.internal_pings.deletion_request.submit(&glean, None); let directory_manager = PingDirectoryManager::new(dir.path()); diff --git a/glean-core/src/upload/mod.rs b/glean-core/src/upload/mod.rs index adefc2448b..877acb3d88 100644 --- a/glean-core/src/upload/mod.rs +++ b/glean-core/src/upload/mod.rs @@ -874,14 +874,10 @@ mod test { // Submit the ping multiple times let n = 10; for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } - glean - .internal_pings - .deletion_request - .submit(&glean, None) - .unwrap(); + glean.internal_pings.deletion_request.submit(&glean, None); // Clear the queue drop(glean.upload_manager.clear_ping_queue()); @@ -907,7 +903,7 @@ mod test { // Submit the ping multiple times let n = 10; for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } // Create a new upload manager pointing to the same data_path as the glean instance. @@ -935,7 +931,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit a ping - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); // Get the pending ping directory path let pending_pings_dir = dir.path().join(PENDING_PINGS_DIRECTORY); @@ -965,7 +961,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit a ping - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); // Get the pending ping directory path let pending_pings_dir = dir.path().join(PENDING_PINGS_DIRECTORY); @@ -995,7 +991,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit a ping - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); // Get the submitted PingRequest match glean.get_upload_task() { @@ -1027,7 +1023,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit a ping - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); // Get the pending ping directory path let pending_pings_dir = dir.path().join(PENDING_PINGS_DIRECTORY); @@ -1104,7 +1100,7 @@ mod test { glean.register_ping_type(&ping_type); // Submit a ping - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); // Get the submitted PingRequest match glean.get_upload_task() { @@ -1152,7 +1148,7 @@ mod test { // Submit the ping multiple times let n = 5; for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } let mut upload_manager = PingUploadManager::no_policy(dir.path()); @@ -1200,7 +1196,7 @@ mod test { // Submit the ping multiple times let n = 10; for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } let directory_manager = PingDirectoryManager::new(dir.path()); @@ -1272,7 +1268,7 @@ mod test { // Submit the ping multiple times for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } let directory_manager = PingDirectoryManager::new(dir.path()); @@ -1343,7 +1339,7 @@ mod test { // Submit the ping multiple times for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } let directory_manager = PingDirectoryManager::new(dir.path()); @@ -1417,7 +1413,7 @@ mod test { // Submit the ping multiple times for _ in 0..n { - glean.submit_ping(&ping_type, None).unwrap(); + glean.submit_ping(&ping_type, None); } let directory_manager = PingDirectoryManager::new(dir.path()); diff --git a/glean-core/tests/ping.rs b/glean-core/tests/ping.rs index d0437bf963..20d034ff8b 100644 --- a/glean-core/tests/ping.rs +++ b/glean-core/tests/ping.rs @@ -25,7 +25,7 @@ fn write_ping_to_disk() { }); counter.add(&glean, 1); - assert!(ping.submit(&glean, None).unwrap()); + assert!(ping.submit(&glean, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -46,7 +46,7 @@ fn disabling_upload_clears_pending_pings() { }); counter.add(&glean, 1); - assert!(ping.submit(&glean, None).unwrap()); + assert!(ping.submit(&glean, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // At this point no deletion_request ping should exist // (that is: it's directory should not exist at all) @@ -69,7 +69,7 @@ fn disabling_upload_clears_pending_pings() { assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); counter.add(&glean, 1); - assert!(ping.submit(&glean, None).unwrap()); + assert!(ping.submit(&glean, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -111,11 +111,11 @@ fn empty_pings_with_flag_are_sent() { // No data is stored in either of the custom pings // Sending this should succeed. - assert_eq!(true, ping1.submit(&glean, None).unwrap()); + assert_eq!(true, ping1.submit(&glean, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // Sending this should fail. - assert_eq!(false, ping2.submit(&glean, None).unwrap()); + assert_eq!(false, ping2.submit(&glean, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -152,7 +152,7 @@ fn test_pings_submitted_metric() { }); counter.add(&glean, 1); - assert!(metrics_ping.submit(&glean, None).unwrap()); + assert!(metrics_ping.submit(&glean, None)); // Check recording in the metrics ping assert_eq!( @@ -186,8 +186,8 @@ fn test_pings_submitted_metric() { // This should record a count of 2 baseline pings in the metrics ping, but // it resets each time on the baseline ping, so we should only ever get 1 // baseline ping recorded in the baseline ping itsef. - assert!(baseline_ping.submit(&glean, None).unwrap()); - assert!(baseline_ping.submit(&glean, None).unwrap()); + assert!(baseline_ping.submit(&glean, None)); + assert!(baseline_ping.submit(&glean, None)); // Check recording in the metrics ping assert_eq!( diff --git a/glean-core/tests/ping_maker.rs b/glean-core/tests/ping_maker.rs index ec3e5e1225..8781ad494d 100644 --- a/glean-core/tests/ping_maker.rs +++ b/glean-core/tests/ping_maker.rs @@ -177,7 +177,7 @@ fn clear_pending_pings() { }); metric.set(&glean, true); - assert!(glean.submit_ping(&ping_type, None).is_ok()); + assert!(glean.submit_ping(&ping_type, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); assert!(ping_maker @@ -194,17 +194,17 @@ fn no_pings_submitted_if_upload_disabled() { let ping_type = PingType::new("store1", true, true, vec![]); glean.register_ping_type(&ping_type); - assert!(glean.submit_ping(&ping_type, None).is_ok()); + assert!(glean.submit_ping(&ping_type, None)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // Disable upload, then try to sumbit glean.set_upload_enabled(false); - assert!(glean.submit_ping(&ping_type, None).is_ok()); + assert!(!glean.submit_ping(&ping_type, None)); assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); // Test again through the direct call - assert!(ping_type.submit(&glean, None).is_ok()); + assert!(!ping_type.submit(&glean, None)); assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -215,7 +215,7 @@ fn metadata_is_correctly_added_when_necessary() { let ping_type = PingType::new("store1", true, true, vec![]); glean.register_ping_type(&ping_type); - assert!(glean.submit_ping(&ping_type, None).is_ok()); + assert!(glean.submit_ping(&ping_type, None)); let (_, _, metadata) = &get_queued_pings(glean.get_data_path()).unwrap()[0]; let headers = metadata.as_ref().unwrap().get("headers").unwrap(); From 1bf066e402a056971280f46df6cf7e1674dd3033 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Tue, 4 May 2021 15:11:10 +0200 Subject: [PATCH 2/4] Update generated metric documentation Unfortunately there's a left-over "DO NOT COMMIT" in there. Fix is pending upstream: https://github.com/mozilla/glean_parser/pull/322 --- docs/user/user/collected-metrics/metrics.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user/user/collected-metrics/metrics.md b/docs/user/user/collected-metrics/metrics.md index 0a70ad33c8..33245d567a 100644 --- a/docs/user/user/collected-metrics/metrics.md +++ b/docs/user/user/collected-metrics/metrics.md @@ -1,4 +1,4 @@ - + # Metrics @@ -89,7 +89,7 @@ This ping includes the [client id](https://mozilla.github.io/glean/book/user/pin **Data reviews for this ping:** - -- +- **Bugs related to this ping:** @@ -168,5 +168,5 @@ In addition to those built-in metrics, the following metrics are added to the pi Data categories are [defined here](https://wiki.mozilla.org/Firefox/Data_Collection). - + From b18526d5b3fb35a7159066be728463f877d40f5b Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Tue, 4 May 2021 15:00:52 +0200 Subject: [PATCH 3/4] Log informational line when handling active/inactive Submitting can't hard-fail anymore, so we can reduce our warning to info here. --- glean-core/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index b22b782fe6..82bd521b1c 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -923,7 +923,7 @@ impl Glean { /// and then sets the dirty bit. pub fn handle_client_active(&mut self) { if !self.internal_pings.baseline.submit(self, Some("active")) { - log::warn!("Failed to submit baseline ping on active"); + log::info!("baseline ping not submitted on active"); } self.set_dirty_flag(true); @@ -935,11 +935,11 @@ impl Glean { /// `inactive` and then clears the dirty bit. pub fn handle_client_inactive(&mut self) { if !self.internal_pings.baseline.submit(self, Some("inactive")) { - log::warn!("Failed to submit baseline ping on inactive"); + log::info!("baseline ping not submitted on inactive"); } if !self.internal_pings.events.submit(self, Some("inactive")) { - log::warn!("Failed to submit events ping on inactive"); + log::info!("events ping not submitted on inactive"); } self.set_dirty_flag(false); From e652433fbae1d82e46e1eb1be062cb1ebd08360e Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Tue, 4 May 2021 15:06:49 +0200 Subject: [PATCH 4/4] Kotlin: ensure test triggers _some_ work This is required because of the changes made to ping.submit before and how it was used internally. Previously in event_database, when triggering startup event pings, we only checked if storing a ping did not fail. We did not check if any ping was actually submitted. So the code around it launched a task on the workmanager, which then subsequently checked and did nothing. The test ensured that, and triggering the work manager was just fine because there _was_ a pending task. With the changes we now return `true` if any ping was submitted. Or `false` if no event ping was submitted on startup. Now on startup no task was triggered and thus no workmanager can be triggered. That made the test fail because `triggerWorkManager` requires a task to be enqueued. We can force a task by also recording into the builtin "events" ping. It's a small hack, but allows us to test that late-registered pings with events are correctly skipped AND their events are deleted. --- .../mozilla/telemetry/glean/private/EventMetricTypeTest.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt index 89b3deb320..95f3efc558 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt @@ -495,7 +495,7 @@ class EventMetricTypeTest { category = "telemetry", name = "test_event", lifetime = Lifetime.Ping, - sendInPings = listOf(pingName), + sendInPings = listOf(pingName, "events"), // also send in builtin ping allowedExtraKeys = listOf("someExtra") ) @@ -523,11 +523,14 @@ class EventMetricTypeTest { sendIfEmpty = false, reasonCodes = listOf()) - // Trigger worker task to upload the pings in the background + // Trigger worker task to upload the pings in the background. + // Because this also triggers the builtin "events" ping + // we should definitely get _something_. triggerWorkManager(context) // We can't properly test the absence of data, // but we can try to receive one and that causes an exception if there is none. + // We also get the "events" ping, which we'll simply ignore here. assertNull(waitForPingContent(pingName, null, server)) // Now try to manually submit the ping.