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.

Sign up for GitHub

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

Jump to bottom

test(deck-options): discard changes dialog #17851

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

test(deck-options): discard changes dialog #17851

wants to merge 2 commits into from
+225 −3

Conversation

david-allison
Copy link
Member

Purpose / Description

@Arthur-Milchior said this would be hard. I knew he was right, and I did it anyway. I would regret my time spent here, but it will hopefully lead to more regression coverage for our backend pages

Issue

Approach

How Has This Been Tested?

  • My personal S21 (Android 14)
  • API 30 emulator

Warning

This needs a number of reruns in CI to be sure it's stable

Learning (optional, can help others)

This is the first test of anki.pages, and hopefully will allow us to stop a few 'brown paper bag' bugs in future, such as when we broke Statistics

onView can't be called inside onActivity. There feels like a lot of bad advice here, and the easy option is to move the assertion outside onActivity: https://stackoverflow.com/questions/61953249/how-to-access-activity-from-activityscenariorule

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@david-allison
test(deck-options): no 'discard?' dialog initially
d309d2a 
Mostly to get a framework in place for testing
DeckOptions

Some of it will want to be abstracted at a later date

The WebView based timing issues are painful.
As is 'actuallyClose', which launches a task to close

10ms was sufficient for the WebView, but I wanted some leeway
for CI

`isClosingFragment` seemed easier than waiting on tasks,
even if less architecturally pure. It also means we
listen to task start, rather than task finish

Issue 14438
@david-allison
test(deck-options): discard dialog if changes made
6f5452b 
Issue 14438

----

This was a difficult change to make:

* timing considerations on WebView callbacks

I spent time time trying to use `onView` inside `onActivity`.

After research, `onView` should be outside the `onActivity`
hook: https://stackoverflow.com/a/64284563
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@david-allison