Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

enha: make root checker better and minimize false positive #417

Merged
merged 7 commits into from May 25, 2020
Merged

enha: make root checker better and minimize false positive #417

merged 7 commits into from May 25, 2020

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented May 17, 2020

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

馃摐 Description

make root checker better and minimize false positive.

馃挕 Motivation and Context

I still could reproduce false positive on the current root checker

馃挌 How did you test it?

馃摑 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

馃敭 Next steps

@codecov-io
Copy link

codecov-io commented May 18, 2020

Codecov Report

Merging #417 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #417   +/-   ##
=========================================
  Coverage     60.18%   60.18%           
  Complexity      809      809           
=========================================
  Files            92       92           
  Lines          3747     3747           
  Branches        360      360           
=========================================
  Hits           2255     2255           
  Misses         1338     1338           
  Partials        154      154           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 51cd51e...12fb5a5. Read the comment docs.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

The code looks good, but wouldn't it be better to add tests for it?

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2020

Codecov Report

Merging #417 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #417   +/-   ##
=========================================
  Coverage     59.96%   59.96%           
  Complexity      812      812           
=========================================
  Files            93       93           
  Lines          3742     3742           
  Branches        363      363           
=========================================
  Hits           2244     2244           
  Misses         1343     1343           
  Partials        155      155           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 445e83f...3dc5e76. Read the comment docs.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

I really like it way more now. It would be super nice to just have one integration test on a real android device or a virtual device just to make sure everything is wired up properly, but that's up to you.

@marandaneto marandaneto merged commit 59f6ab0 into getsentry:master May 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants