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

Android P compatibility changes #350

Merged
merged 4 commits into from
Jul 27, 2018
Merged

Android P compatibility changes #350

merged 4 commits into from
Jul 27, 2018

Conversation

fractalwrench
Copy link
Contributor

Goal

Test compatibility with Android P preview and make any necessary fixes.

Changeset

  • Make StrictMode check insensitive
  • Alter the test assertion on P devices for StrictMode. The exception message has changed so that we can't parse the violation code, meaning a full description isn't possible to obtain, although we can still detect that some issue occurred.

Tests

  • Local unit tests

  • Manual tests with dashboard

  • Read the new features + changes to find any relevant changes https://android-developers.googleblog.com/2018/07/final-preview-update-official-android-p.html

  • Mazerunner scenarios. It's worth noting that unhandled scenarios don't pass because P is more strict about killing the app after an exception, and the notifier would now deliver reports on the next app launch. As the scenarios fail due to incorrect assumptions about the report, I've left them as-is until we get to a point where mazerunner is capable of verifying multiple API levels at the same time.

Review

For the submitter, initial self-review:

  • Commented on code changes inline explain the reasoning behind the approach
  • Reviewed the test cases added for completeness and possible points for discussion
  • A changelog entry was added for the goal of this pull request
  • Check the scope of the changeset - is everything in the diff required for the pull request?
  • This pull request is ready for:
    • Initial review of the intended approach, not yet feature complete
    • Structural review of the classes, functions, and properties modified
    • Final review

For the pull request reviewer(s), this changeset has been reviewed for:

  • Consistency across platforms for structures or concepts added or modified
  • Consistency between the changeset and the goal stated above
  • Internal consistency with the rest of the library - is there any overlap between existing interfaces and any which have been added?
  • Usage friction - is the proposed change in usage cumbersome or complicated?
  • Performance and complexity - are there any cases of unexpected O(n^3) when iterating, recursing, flat mapping, etc?
  • Concurrency concerns - if components are accessed asynchronously, what issues will arise
  • Thoroughness of added tests and any missing edge cases
  • Idiomatic use of the language

In the latest P preview, the android.os.strictmode class name is all lower case. This change updates
the check to be case insensitive
The strictmode exception message for P changed so that it doesn't include the policy violation code,
meaning we are unable to parse the necessary information. This updates the test to ensure that we
gracefully fallback to providing null as the description.
@fractalwrench fractalwrench requested a review from a team July 26, 2018 14:12
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 74.927% when pulling 36df875 on android-p-compat into 397d771 on next.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 74.927% when pulling 36df875 on android-p-compat into 397d771 on next.

@coveralls
Copy link

coveralls commented Jul 26, 2018

Coverage Status

Coverage decreased (-0.09%) to 74.927% when pulling 36df875 on android-p-compat into 397d771 on next.

@fractalwrench fractalwrench merged commit 4a8588b into next Jul 27, 2018
@fractalwrench fractalwrench deleted the android-p-compat branch July 27, 2018 08:13
lemnik pushed a commit that referenced this pull request Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants