Skip to content
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

Stack trace linking for VSTS #52954

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Stack trace linking for VSTS #52954

merged 4 commits into from
Sep 29, 2023

Conversation

jianyuan
Copy link
Contributor

@jianyuan jianyuan commented Jul 16, 2023

This PR enables Stack trace linking for VSTS.

Depends on #53584

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 16, 2023
@codecov
Copy link

codecov bot commented Jul 16, 2023

Codecov Report

Merging #52954 (21678ae) into master (0edd435) will increase coverage by 0.53%.
The diff coverage is 100.00%.

❗ Current head 21678ae differs from pull request most recent head 348eaa9. Consider uploading reports for the commit 348eaa9 to get more accurate results

@@            Coverage Diff             @@
##           master   #52954      +/-   ##
==========================================
+ Coverage   78.84%   79.37%   +0.53%     
==========================================
  Files        5086     4934     -152     
  Lines      219350   207248   -12102     
  Branches    37160    35406    -1754     
==========================================
- Hits       172942   164507    -8435     
+ Misses      40806    37710    -3096     
+ Partials     5602     5031     -571     
Files Changed Coverage
src/sentry/integrations/mixins/repositories.py ø
src/sentry/integrations/vsts/client.py 100.00%
src/sentry/integrations/vsts/integration.py 100.00%

Copy link
Member

@leeandher leeandher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Could we also get a test for the project_stacktrace_link endpoint that VSTS links are added to the payload? That is how we can get these links to the frontend.

@jianyuan jianyuan marked this pull request as ready for review September 24, 2023 23:21
@jianyuan jianyuan requested a review from a team as a code owner September 24, 2023 23:21
@jianyuan
Copy link
Contributor Author

@leeandher The test_get_stacktrace_link test should cover that particular use case.

The project_stacktrace_link endpoint invokes the get_stacktrace_link method here. The existing tests for this endpoint utilise the ExampleIntegration for all their tests rather than a specific integration. Moreover, these tests patch the get_stacktrace_link method to return the URL that the frontend displays.

@leeandher
Copy link
Member

Looks great! Tested it locally with no issues! I'll run the CI and merge this early next week 👍
image

@vercel
Copy link

vercel bot commented Sep 29, 2023

@leeandher is attempting to deploy a commit to the Sentry Team on Vercel.

A member of the Team first needs to authorize it.

@leeandher leeandher added the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Sep 29, 2023
@leeandher
Copy link
Member

/gcbrun

@leeandher leeandher added Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests and removed Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests labels Sep 29, 2023
@leeandher leeandher merged commit e8c2b61 into getsentry:master Sep 29, 2023
50 of 53 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants