The Gecko Profiler's test coverage is pretty bad. Bug 1348776 is a good example of something that should have been discovered via automated testing. A gtest can test most or all of the profiler's public functions, i.e. profiler_*().

This required a tweak to DoNativeBacktrace() to work around an ASAN false positive.

Comment on attachment 8853234 [details] [diff] [review] Write a gtest for basic features of the Gecko Profiler Review of attachment 8853234 [details] [diff] [review]: ----------------------------------------------------------------- This is great. Please remember to remove the calls to the (now deleted) function profiler_is_active_and_not_in_privacy_mode() before landing this. ::: tools/profiler/tests/gtest/GeckoProfiler.cpp @@ +376,5 @@ > + profiler_start(PROFILE_DEFAULT_ENTRIES, PROFILE_DEFAULT_INTERVAL, > + features, MOZ_ARRAY_LENGTH(features), > + filters, MOZ_ARRAY_LENGTH(filters)); > + > + UniqueProfilerBacktrace bt = profiler_get_backtrace(); You're not doing anything with bt. Is this an oversight, or are you just testing that this doesn't crash? @@ +385,5 @@ > + SamplerStackFrameDynamicRAII raii2("A", js::ProfileEntry::Category::STORAGE, > + 888, dynamic.get()); > + void* handle = profiler_call_enter("A", js::ProfileEntry::Category::NETWORK, > + this, false, 999); > + UniqueProfilerBacktrace bt = profiler_get_backtrace(); Same here. @@ +386,5 @@ > + 888, dynamic.get()); > + void* handle = profiler_call_enter("A", js::ProfileEntry::Category::NETWORK, > + this, false, 999); > + UniqueProfilerBacktrace bt = profiler_get_backtrace(); > + profiler_call_exit(handle); I'd prefer to call these stack classes and functions "implementation details" and not consider them part of the API. Not sure why you're testing them separately after testing the PROFILER_LABEL* macro versions.

> Please remember to remove the calls to the (now deleted) function > profiler_is_active_and_not_in_privacy_mode() before landing this. Done. > You're not doing anything with bt. Is this an oversight, or are you just > testing that this doesn't crash? Basically just testing that it doesn't crash, but I changed both occurrences to ASSERT_TRUE(profiler_get_backtrace()) to make them consistent with other calls. > I'd prefer to call these stack classes and functions "implementation > details" and not consider them part of the API. Not sure why you're testing > them separately after testing the PROFILER_LABEL* macro versions. Well, if they're in GeckoProfiler.h, I want to test them separately.

https://hg.mozilla.org/integration/mozilla-inbound/rev/967edd49939b0f42cfddbe488ca9b7044c2b4f53 Bug 1351136 - Write a gtest for basic features of the Gecko Profiler. r=mstange.

https://hg.mozilla.org/integration/mozilla-inbound/rev/abdb79a72b8177377404ddbc6d9d06c0ffa8f07c Bug 1351136 (follow-up) - Fix a harmless argument mis-ordering in a call to profiler_get_start_params(). r=me.