tests(derived_code_mappings): Refactor the test a bit #86189
Conversation
Changes included: * Clearly show what frame is used within a specific test * Force calling helper functions with keyword arguments
| """Helper function to prevent creating an event without a platform.""" | ||
| test_data = {"platform": platform, "stacktrace": {"frames": frames}} | ||
| return self.store_event(data=test_data, project_id=self.project.id) | ||
| # XXX: In the future fix store_event to return the correct type | ||
| return cast(GroupEvent, self.store_event(data=test_data, project_id=self.project.id)) |
There was a problem hiding this comment.
Returning a GroupEvent guarantees that group_id is always defined. Without this change, this line would complain with group_id not always being defined.
| patch("sentry.utils.metrics.incr") as mock_incr, | ||
| ): | ||
| process_event(self.project.id, self.event.group_id, self.event.event_id) | ||
| event = self.create_event(frames, platform) |
There was a problem hiding this comment.
This event creation used to happen here:
| assert not RepositoryProjectPathConfig.objects.exists() | ||
| return code_mappings | ||
|
|
||
| def frame(self, filename: str, in_app: bool | None = True) -> dict[str, str | bool]: |
There was a problem hiding this comment.
New helper function. Not necessary but it helps shorten lines.
| platform: str, | ||
| ) -> list[CodeMapping]: | ||
| with patch(GET_TREES_FOR_ORG, return_value=self._get_trees_for_org(repo_files)): | ||
| event = self.create_event(frames, platform) |
There was a problem hiding this comment.
Creating the event here rather than in the setUp function.
| {"in_app": True, "filename": "\\sentry\\dog\\cat\\parrot.py"}, | ||
| {"in_app": True, "filename": "C:sentry\\tasks.py"}, | ||
| {"in_app": True, "filename": "D:\\Users\\code\\sentry\\models\\release.py"}, | ||
| ] |
There was a problem hiding this comment.
Up until now, the frames would be defined at the class level rather than within the test.
The tests will now clearly state which of all frames will be in use (rather than the whole list defined here):
There was a problem hiding this comment.
This PR now forces us to declare what frames are considered within each test rather than having a list at the top of the class.
armenzg
added this to the Automate Code Mappings & In-App Stack Trace Rules for Java projects milestone
Changes included:
This forces writing tests by declaring frames within the test rather than having a list at the top of the class.
