-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
test(derived_code_mappings): Prevent not updating the list of extensions #86196
test(derived_code_mappings): Prevent not updating the list of extensions #86196
Conversation
This change mocks the code at a lower level making sure that when we write tests it will fail until the list of supported extensions is updated.
@@ -124,10 +124,12 @@ def _populate_trees(self, repositories: Sequence[dict[str, str]]) -> dict[str, R | |||
remaining_requests = MINIMUM_REQUESTS_REMAINING | |||
try: | |||
remaining_requests = self.get_client().get_remaining_api_requests() | |||
except ApiError: | |||
except Exception: |
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.
This is an orthogonal change.
use_cache = True | ||
# Report so we can investigate | ||
logger.warning("Loading trees from cache. Execution will continue. Check logs.") | ||
logger.warning( | ||
"Loading trees from cache. Execution will continue. Check logs.", exc_info=True |
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.
def _process_and_assert_code_mapping( | ||
self, files: Sequence[str], stack_root: str, source_root: str | ||
) -> None: | ||
with ( | ||
patch(GET_TREES_FOR_ORG, return_value=self._get_trees_for_org(files)), |
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.
Instead of patching at a higher level, we patch three functions deeper into the code base.
sentry/src/sentry/integrations/source_code_management/repo_trees.py
Lines 80 to 88 in c0f7432
def get_trees_for_org(self) -> dict[str, RepoTree]: | |
trees = {} | |
repositories = self._populate_repositories() | |
if not repositories: | |
logger.warning("Fetching repositories returned an empty list.") | |
else: | |
trees = self._populate_trees(repositories) | |
return trees |
def _process_and_assert_code_mapping( | ||
self, files: Sequence[str], stack_root: str, source_root: str | ||
) -> None: | ||
with ( | ||
patch(GET_TREES_FOR_ORG, return_value=self._get_trees_for_org(files)), | ||
patch(f"{CLIENT}.get_tree", return_value=self._repo_tree_files(files)), | ||
patch(f"{CLIENT}.get_remaining_api_requests", return_value=500), |
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.
_populate_trees
will call this:
remaining_requests = self.get_client().get_remaining_api_requests() |
def _process_and_assert_code_mapping( | ||
self, files: Sequence[str], stack_root: str, source_root: str | ||
) -> None: | ||
with ( | ||
patch(GET_TREES_FOR_ORG, return_value=self._get_trees_for_org(files)), | ||
patch(f"{CLIENT}.get_tree", return_value=self._repo_tree_files(files)), |
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.
After we call get_tree
[1] we will call filter_source_code_files
which reduces the number of files to store in the cache based on the extension.
sentry/src/sentry/integrations/source_code_management/repo_trees.py
Lines 198 to 203 in c0f7432
tree = self.get_client().get_tree(repo_full_name, tree_sha) | |
if tree: | |
# Keep files; discard directories | |
repo_files = [x["path"] for x in tree if x["type"] == "blob"] | |
if only_source_code_files: | |
repo_files = filter_source_code_files(files=repo_files) |
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.
At the end of the file, you will see the test that verifies the correctness of the fix.
|
||
with patch(f"{CODE_ROOT}.task.supported_platform", return_value=True): | ||
# No extensions are supported, thus, we can't generate a code mapping | ||
self._process_and_assert_no_code_mapping([file_in_repo]) |
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.
The code mapping does not get generated since the tbd
file extension has not yet been added to the list of supported extensions.
self._process_and_assert_no_code_mapping([file_in_repo]) | ||
|
||
with patch(f"{REPO_TREES_CODE}.SUPPORTED_EXTENSIONS", ["tbd"]): | ||
self._process_and_assert_code_mapping([file_in_repo], "foo/", "src/foo/") |
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.
Now it works! 💪🏻
…ons (#86196) This change mocks the code at a lower level ensuring that tests will fail until the list of supported extensions is updated.
This change mocks the code at a lower level ensuring that tests will fail until the list of supported extensions is updated.