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

fix: js api fetch error in card template viewer and previewer #15520

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

krmanik
Copy link
Member

@krmanik krmanik commented Feb 16, 2024

Purpose / Description

The JS API is not able to fetch request in Card Template Viewer and Previewer because of there is no instance of server.

Fixes

  • Fixes Link to the issues.

Approach

Moved the AnkiServer to AbstractFlashcardViewer, so it will be available to Reviewer, Previewer and Card Template Viewer.

How Has This Been Tested?

Tested on Emulator with this Anki Deck

AnkiDroid JS API Test.zip

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

@krmanik krmanik force-pushed the fix-js-api branch 2 times, most recently from 1ceb604 to b64d862 Compare February 16, 2024 10:19
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

LGTM, nitpicks only

@david-allison david-allison added the Needs Second Approval Has one approval, one more approval to merge label Feb 16, 2024
@mikehardy
Copy link
Member

🤔 well this is a new macOS flake, never seen this before:


                at com.ichi2.anki.dialogs.CreateDeckDialogTest.setUp(CreateDeckDialogTest.kt:52)

                Caused by:
                java.lang.OutOfMemoryError: Java heap space
                    at java.desktop/java.awt.image.DataBufferInt.<init>(DataBufferInt.java:77)
                    at java.desktop/java.awt.image.Raster.createPackedRaster(Raster.java:538)
                    at java.desktop/java.awt.image.DirectColorModel.createCompatibleWritableRaster(DirectColorModel.java:1032)
                    at java.desktop/java.awt.image.BufferedImage.<init>(BufferedImage.java:333)
                    at org.robolectric.shadows.ShadowBitmapFactory.create(ShadowBitmapFactory.java:287)
                    at org.robolectric.shadows.ShadowBitmapFactory.decodeStream(ShadowBitmapFactory.java:189)
                    at org.robolectric.shadows.ShadowBitmapFactory.decodeStream(ShadowBitmapFactory.java:153)
                    at 

@mikehardy
Copy link
Member

Again!! What in the world ?


                at com.ichi2.anki.dialogs.CreateDeckDialogTest.setUp(CreateDeckDialogTest.kt:52)

                Caused by:
                java.lang.OutOfMemoryError: Java heap space
                    at java.desktop/java.awt.image.DataBufferInt.<init>(DataBufferInt.java:77)
                    at java.desktop/java.awt.image.Raster.createPackedRaster(Raster.java:538)
                    at java.desktop/java.awt.image.DirectColorModel.createCompatibleWritableRaster(DirectColorModel.java:1032)
                    at java.desktop/java.awt.image.BufferedImage.<init>(BufferedImage.java:333)
                    at org.robolectric.shadows.ShadowBitmapFactory.create(ShadowBitmapFactory.java:287)

macOS only

@mikehardy
Copy link
Member

Maybe we need a tearDown that pushes the activity lifecycle through stopped and terminated or something?

override fun setUp() {
super.setUp()
getPreferences().edit { putBoolean(IntroductionActivity.INTRODUCTION_SLIDES_SHOWN, true) }
ensureCollectionLoadIsSynchronous()
activityScenario = ActivityScenario.launch(DeckPicker::class.java).apply {
moveToState(Lifecycle.State.STARTED)
}
}

It's failing on the second test here, like the first one passes but the second one is too much. And of course only one OS.

Alternative solution, peel this flake to a new issue and (assuming the Flaky annotation supports classes and not just methods?) is to add a @Flaky(OS.MACOS, "Issue NNNN layout inflation fails with OOM on macOS") annotation on the class ?

@david-allison david-allison added the Blocked by dependency Currently blocked by some other dependent / related change label Feb 16, 2024
@david-allison
Copy link
Member

@david-allison david-allison removed the Blocked by dependency Currently blocked by some other dependent / related change label Feb 16, 2024
@david-allison
Copy link
Member

david-allison commented Feb 16, 2024

ubuntu failure

https://github.com/ankidroid/Anki-Android/actions/runs/7932349193/job/21658645926?pr=15520

* What went wrong:
Execution failed for task ':AnkiDroid:testPlayDebugUnitTest'.
> A build operation failed.
      Immutable workspace contents have been modified: /home/runner/.gradle/caches/transforms-4/f46cf63d0a549c8bc814e25133f449eb. These workspace directories are not supposed to be modified once they are created. Deleting the directory in question can allow the content to be recreated.
   > Immutable workspace contents have been modified: /home/runner/.gradle/caches/transforms-4/f46cf63d0a549c8bc814e25133f449eb. These workspace directories are not supposed to be modified once they are created. Deleting the directory in question can allow the content to be recreated.

@david-allison david-allison added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Feb 16, 2024
@mikehardy
Copy link
Member

This PR is cursed but it is not anyone's fault. Ubuntu is now showing the gradle error that led me to disable gradle caching for windows unit test runner yesterday:


Execution failed for task ':AnkiDroid:testPlayDebugUnitTest'.
> A build operation failed.
      Immutable workspace contents have been modified: /home/runner/.gradle/caches/transforms-4/f46cf63d0a549c8bc814e25133f449eb. These workspace directories are not supposed to be modified once they are created. Deleting the directory in question can allow the content to be recreated.
   > Immutable workspace contents have been modified: /home/runner/.gradle/caches/transforms-4/f46cf63d0a549c8bc814e25133f449eb. These workspace directories are not supposed to be modified once they are created. Deleting the directory in question can allow the content to be recreated.

The effect of disabling cache was that it was a few minutes slower for the unit test run but that it was reliable.

This is unfortunate. I will purge the cache temporarily in order to get this PR through then consider disabling caching entirely until I/we figure out what happened here.

It may be the switch to gradle 8.6 as I think that was the most recent change.

Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

LGTM, FWIW

@david-allison david-allison added this pull request to the merge queue Feb 16, 2024
@mikehardy
Copy link
Member

logged upstream issue for gradle transform cache corruption / immutability incompatibility gradle/actions#47

Merged via the queue into ankidroid:main with commit b0a4c27 Feb 16, 2024
7 checks passed
@github-actions github-actions bot added this to the 2.17 release milestone Feb 16, 2024
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Feb 16, 2024
Copy link
Contributor

Hi there @krmanik! This is the OpenCollective Notice for PRs merged from 2024-02-01 through 2024-02-29

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

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