Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[a11y] Improve EmbedderA11yTest, fixture comments #40995

Merged
merged 1 commit into from
Apr 8, 2023

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Apr 7, 2023

Applies cleanup/simplification to EmbedderA11yTest unit tests.

  • Add numbered breadcrumbs to identify corresponding sections of the C++ and Dart fixture tests.
  • Eliminates duplicated native method implementations for notifying:
    • whether semantics is enabled
    • whether a particular accessibility feature is enabled
    • which semantics action was fired

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label Apr 7, 2023
@cbracken cbracken force-pushed the embedder-a11y-unittest-breadcrumbs branch from 86e8893 to afe954d Compare April 7, 2023 20:04
@cbracken cbracken requested a review from loic-sharma April 7, 2023 20:09
@cbracken
Copy link
Member Author

cbracken commented Apr 7, 2023

This is stacked on top of #40993 the relevant diffs are in the second commit in this PR.

@cbracken cbracken changed the title [a11y] Add breadcrumbs in unittests comments [a11y] Add breadcrumbs in unittest comments Apr 7, 2023
@cbracken cbracken force-pushed the embedder-a11y-unittest-breadcrumbs branch from afe954d to 5cf8df7 Compare April 7, 2023 20:12
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! 🏆

@cbracken cbracken force-pushed the embedder-a11y-unittest-breadcrumbs branch 2 times, most recently from a4e5336 to 22152af Compare April 7, 2023 23:36
@cbracken cbracken changed the title [a11y] Add breadcrumbs in unittest comments [a11y] Simplify EmbedderA11yTest tests Apr 7, 2023
@cbracken cbracken changed the title [a11y] Simplify EmbedderA11yTest tests WIP: [a11y] Simplify EmbedderA11yTest tests Apr 8, 2023
@cbracken cbracken added the Work in progress (WIP) Not ready (yet) for review! label Apr 8, 2023
@cbracken cbracken force-pushed the embedder-a11y-unittest-breadcrumbs branch from 603f6a7 to c29057a Compare April 8, 2023 02:54
@cbracken
Copy link
Member Author

cbracken commented Apr 8, 2023

Alright I've reverted back to just cleaning up the comments as it was originally. I'll move the other refactoring to another patch.

Add breadcrumbs to make it easier to jump between the Dart fixture side
and the embedder side of the EmbedderA11yTest unit test.
@cbracken cbracken force-pushed the embedder-a11y-unittest-breadcrumbs branch from c29057a to fe16f78 Compare April 8, 2023 02:56
@cbracken cbracken changed the title WIP: [a11y] Simplify EmbedderA11yTest tests [a11y] Improve EmbedderA11yTest, fixture comments Apr 8, 2023
@cbracken cbracken merged commit 54b3284 into flutter:main Apr 8, 2023
@cbracken cbracken deleted the embedder-a11y-unittest-breadcrumbs branch April 8, 2023 17:25
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 8, 2023
zhongwuzw pushed a commit to zhongwuzw/engine that referenced this pull request Apr 14, 2023
Add breadcrumbs to make it easier to jump between the Dart fixture side
and the embedder side of the EmbedderA11yTest unit test.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
embedder Related to the embedder API Work in progress (WIP) Not ready (yet) for review!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants