-
Notifications
You must be signed in to change notification settings - Fork 596
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 Harness] Add test harness library #1392
Conversation
Want to add isRound? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an issue template to .github/ISSUE_TEMPLATE and also an entry for the docs generation into mkdocs.yml
Also, is there no sample for this project?
For For example: if you render something in the very corner of a physically round watch (such that it isn't actually visible), do the testing APIs treat it as being visible? If they do, the |
d6100a5
to
ffb84cd
Compare
Added, along with a first draft of documentation as well.
There are the tests of the Do you think it makes sense to add some tests to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Amazing work.
For the sample app, if you don't want to add a test in the sample app, just a note in your docs about where to see a sample in the repo would be good.
Yep, I can follow up. The answer is yes, they can draw outside the bounds and screenshot tools show content outside. So for Paparazzi I change both the configuration as you suggest, but also apply a circular crop. |
I've pushed it anyway, but happy to revert. |
I tried this to see if it could replace my bespoke code. Not sure if it's AGP or paparazzi related (I doubt given it's debug code, not test). But seeing
Hopefully some typo in my copy/paste/simplification. But wanted to flag. |
I can't think of anything that the harness might be doing that might be affecting resource loading. Can you share your code? |
Don't block on this, mainly flagging in case it's a more general problem. |
For completeness I would suggest one of the tests being to change the locale and call stringResource() from a composable. |
I was trying out I believe the issue is the same, caused by this line in layoutlib.
A proper fix would likely be adding some functionality to layoutlib, to support |
List of updates since your approval @bentrengrove :
|
Got the Trengrove 🚀 of approval offline, merging! |
// Update the locale list | ||
if (Build.VERSION.SDK_INT >= 24) { | ||
setLocales(LocaleList.forLanguageTags(locales.toLanguageTags())) | ||
} else { | ||
setLocale(locales[0]) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexvanyo java.util.Locale.setDefault(locales[0])
would be also useful here since java formatters won't know about the locale update otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be interested to hear your use case here, since I'd intentionally prefer to avoid calling java.util.Locale.setDefault(locales[0])
if possible.
java.util.Locale.getDefault()
isn't observable by Compose, so if you're calling that from a @Composable
function, you might not be notified if the locale changes in the general case (for instance, if opting out of activity recreation for locale changes)
If possible, can you retrieve the locale from LocalConfiguration.current.locales
instead to create the formatter?
Adds a new testing library,
testharness
for testing Compose layouts.