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

test: fix studio task recognition for jacoco test tasks #17830

Merged

Conversation

mikehardy
Copy link
Member

@mikehardy mikehardy commented Jan 16, 2025

Purpose / Description

if you do the task registration "bare" without assigning to a variable, Android Studio will recognize your tasks as run targets for UI usage

Fixes

Nothing logged, was question from Arthur in Discord

Approach

refactoring post-definition configuration into the task definition itself, so no local variable is needed and it's just a "bare" task creation

How Has This Been Tested?

We have the play icon next to the targets now:

image

Learning (optional, can help others)

Meh, IDEs are weird. Should have recognized the task registration without this change

Checklist

Please, go through these checks before submitting the PR.

  • 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

if you do the task registration "bare" without assigning to a variable,
Android Studio will recognize your tasks as run targets for UI usage

enable this by refactoring post-definition configuration into the task
definition itself, so no local variable is needed and it's just a "bare"
task creation
@Arthur-Milchior
Copy link
Member

By any chance, is there a way to also be able to just run all of them at once?
In tests, we can right click the test folder or file and run all of thems. Here it only offers to run ankidroid.

When I tried to run through the green arrow on jacocoTestReport, I got

FAILURE: Build completed with 2 failures.

1: Task failed with an exception.

  • What went wrong:
    Execution failed for task ':AnkiDroid:testPlayDebugUnitTest'.

There were failing tests. See the report at: file:///home/milchior/ankidroid/AnkiDroid/build/reports/tests/testPlayDebugUnitTest/index.html

  • Try:

Run with --scan to get full insights.
==============================================================================

2: Task failed with an exception.

  • What went wrong:
    Execution failed for task ':AnkiDroid:connectedPlayDebugAndroidTest'.

com.android.builder.testing.api.DeviceException: No connected devices!

  • Try:

Run with --stacktrace option to get the stack trace.
Run with --info or --debug option to get more log output.
Run with --scan to get full insights.
Get more help at https://help.gradle.org.
==============================================================================

BUILD FAILED in 1m 53s

I guess it's because all simulators are off.
Do you know whether there is a way to ensure it starts a simulator, the way the android-test folders do, please?

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.

This is one of those times I wish I knew gradle better. Great improvement! 🙏

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

mikehardy commented Jan 16, 2025

@Arthur-Milchior

By any chance, is there a way to also be able to just run all of them at once?

When you run the jacocoTestReport task, it runs them all at once (well, via the gradle test runner, but it will go parallel executing individual tests when it is possible). That's the only way I know to do that one

I guess it's because all simulators are off.

Yep, exactly that

Do you know whether there is a way to ensure it starts a simulator, the way the android-test folders do, please?

I do not, sorry - going to go for "incremental improvement" here and say it could be solved with documentation "I have started an android emulator and run ..." for the PR checkbox text ?

Also it could be that having the text say "all tests pass locally (you can launch an emulator then right-click on the androidTest and test directories and 'Run all' in Anki-Android to verify this)"

At which point this PR doesn't really affect anyone but it does at least enable people to run them from the GUI

Copy link
Member

@BrayanDSO BrayanDSO left a comment

Choose a reason for hiding this comment

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

Also LGTM.

@Arthur-Milchior
Copy link
Member

I'm all for incremental improvement. Clearly not blocking. If by any luck it was an easy one-line change, I'd have loved it, but this is certainly a great PR to merge as-is!

@Arthur-Milchior Arthur-Milchior added this pull request to the merge queue Jan 16, 2025
Merged via the queue into ankidroid:main with commit 9c8e6d8 Jan 16, 2025
12 checks passed
@github-actions github-actions bot removed the Needs Second Approval Has one approval, one more approval to merge label Jan 16, 2025
@github-actions github-actions bot added this to the 2.21 release milestone Jan 16, 2025
@mikehardy mikehardy deleted the studio-run-icons-on-jacoco-tasks branch January 17, 2025 19:16
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.

4 participants