-
-
Notifications
You must be signed in to change notification settings - Fork 330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref: profile sample mocking #3133
Merged
Merged
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
ba971cb
extract implementation of SentryProfilerState; extract SentrySample
armcknight c88d79b
move function prototype
armcknight 4c2017f
rename Profiling -> Profiler
armcknight f7bfb88
fix path
armcknight d1d8ac3
wip extracting profile backtrace mocking from objc tests so it can be…
armcknight 05ffde0
wip replacing live backtrace gathering with mocks in swift tests
armcknight 9c41f30
fixup! wip replacing live backtrace gathering with mocks in swift tests
armcknight 8a683e0
fix tests
armcknight d1ebb8e
remove scheme needed for debugging
armcknight 0e1274e
remove unused interface
armcknight cc01579
fix tvos build
armcknight 6679a96
fix tvos build again
armcknight afa1027
for real though, fix tvos builds
armcknight 905cf12
split test and private header
armcknight dea1676
remove unnecessary stall loop
armcknight b9c22e6
Merge remote-tracking branch 'origin/main' into armcknight/ref/profil…
armcknight File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| #import "SentryProfilingConditionals.h" | ||
| #import <Foundation/Foundation.h> | ||
|
|
||
| #if SENTRY_TARGET_PROFILING_SUPPORTED | ||
|
|
||
| # import "SentryBacktrace.hpp" | ||
|
|
||
| using namespace sentry::profiling; | ||
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| Backtrace mockBacktrace(thread::TIDType threadID, const int threadPriority, | ||
| const char *_Nullable threadName, std::uint64_t queueAddress, std::string queueLabel, | ||
| std::vector<std::uintptr_t> addresses); | ||
|
|
||
| NS_ASSUME_NONNULL_END | ||
|
|
||
| #endif // SENTRY_TARGET_PROFILING_SUPPORTED |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #import "SentryProfilerMocks.h" | ||
|
|
||
| #if SENTRY_TARGET_PROFILING_SUPPORTED | ||
|
|
||
| Backtrace | ||
| mockBacktrace(thread::TIDType threadID, const int threadPriority, const char *threadName, | ||
| std::uint64_t queueAddress, std::string queueLabel, std::vector<std::uintptr_t> addresses) | ||
| { | ||
| ThreadMetadata threadMetadata; | ||
| if (threadName != nullptr) { | ||
| threadMetadata.name = threadName; | ||
| } | ||
| threadMetadata.threadID = threadID; | ||
| threadMetadata.priority = threadPriority; | ||
|
|
||
| QueueMetadata queueMetadata; | ||
| queueMetadata.address = queueAddress; | ||
| queueMetadata.label = std::make_shared<std::string>(queueLabel); | ||
|
|
||
| Backtrace backtrace; | ||
| backtrace.threadMetadata = threadMetadata; | ||
| backtrace.queueMetadata = queueMetadata; | ||
| backtrace.addresses = std::vector<std::uintptr_t>(addresses); | ||
|
|
||
| return backtrace; | ||
| } | ||
|
|
||
| #endif // SENTRY_TARGET_PROFILING_SUPPORTED |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #import "SentryProfilingConditionals.h" | ||
| #import <Foundation/Foundation.h> | ||
|
|
||
| #if SENTRY_TARGET_PROFILING_SUPPORTED | ||
|
|
||
| @class SentryProfilerState; | ||
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| /** | ||
| * This delivers a wrapper around the C++ function to create a mock backtrace for incorporation into | ||
| * profiler state that can be called from Swift tests. | ||
| */ | ||
| @interface SentryProfilerMocksSwiftCompatible : NSObject | ||
|
|
||
| + (void)appendMockBacktraceToState:(SentryProfilerState *)state | ||
| threadID:(uint64_t)threadID | ||
| threadPriority:(const int)threadPriority | ||
| threadName:(nullable NSString *)threadName | ||
| queueAddress:(uint64_t)queueAddress | ||
| queueLabel:(NSString *)queueLabel | ||
| addresses:(NSArray<NSNumber *> *)addresses; | ||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END | ||
|
|
||
| #endif // SENTRY_TARGET_PROFILING_SUPPORTED | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| #import "SentryProfilerMocksSwiftCompatible.h" | ||
|
|
||
| #if SENTRY_TARGET_PROFILING_SUPPORTED | ||
|
|
||
| # import "SentryCurrentDate.h" | ||
| # import "SentryProfilerMocks.h" | ||
| # import "SentryProfilerState+ObjCpp.h" | ||
| # include <vector> | ||
|
|
||
| using namespace std; | ||
|
|
||
| @implementation SentryProfilerMocksSwiftCompatible | ||
|
|
||
| + (void)appendMockBacktraceToState:(SentryProfilerState *)state | ||
| threadID:(uint64_t)threadID | ||
| threadPriority:(const int)threadPriority | ||
| threadName:(nullable NSString *)threadName | ||
| queueAddress:(uint64_t)queueAddress | ||
| queueLabel:(NSString *)queueLabel | ||
| addresses:(NSArray<NSNumber *> *)addresses | ||
| { | ||
| auto backtraceAddresses = std::vector<std::uintptr_t>(); | ||
|
|
||
| for (NSNumber *address in addresses) { | ||
| backtraceAddresses.push_back(address.unsignedLongLongValue); | ||
| } | ||
|
|
||
| auto backtrace = mockBacktrace(threadID, threadPriority, | ||
| [threadName cStringUsingEncoding:NSUTF8StringEncoding], queueAddress, | ||
| [queueLabel cStringUsingEncoding:NSUTF8StringEncoding], backtraceAddresses); | ||
| backtrace.absoluteTimestamp = SentryCurrentDate.getCurrentDateProvider.systemTime; | ||
| [state appendBacktrace:backtrace]; | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| #endif // SENTRY_TARGET_PROFILING_SUPPORTED |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: Mocks are usually test doubles that simulate the behavior of a real object with the added ability to verify interactions with that object. They are used to set expectations and assert that specific methods or properties of the mocked object are called or accessed correctly during testing. What you define here is a fake. Fakes are simplified implementations of a real object that mimics its behavior to some extent. They are typically used to provide a functional substitute for a real object that is too complex, slow, or unavailable for testing.TLDR; if you want to follow the correct testing naming terms, we should rename all the mocks in this PR to fake, but I'm also fine with keeping it. I don't want to be nitpicking on testing terms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but I think this is more of a mock. I think we should be careful introducing too much terminology (and therefore cognitive overhead) like fixture, double, oracle, stub, mock, fake, dummy, spy, robot... I don't think these are all standardized terms in industry.
Since this is just creating structs to use as input to a real implementation, and not actually providing an alternate implementation of an interface, there is really no doubling going on at all, so in an OOP sense, this is neither mocking nor stubbing (nor faking).
I think the intention of the code gets across so I'm hesitant to change it. We're providing predefined input to the SUT and testing the output of the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me 👍 .