Use qualifier approach for size #704
Conversation
| if (heightDp > 0) { | ||
| val heightPx = (heightDp * density).roundToInt() | ||
| display.setHeight(heightPx) | ||
| qualifiers.add("h${heightDp}dp") |
There was a problem hiding this comment.
@takahirom
I’m a bit skeptical about this.
I can ensure it did not work reliably previously if you just had one of them (either height or width) in your preview or when the height and width were very small (e.g. 30dp), I cannot recall exactly which of those cases. And using the qualifier was indeed the first approach I tried.
But since it broke with Robolectric 4.15, I wonder which changes they’ve made? And why should it now work with this?
There was a problem hiding this comment.
@sergio-sastre
Thank you for your feedback.
The reason I'm using qualifiers here is due to the issue detailed in this comment. Luckily, I haven't released this change yet. Please let me know if you have any concerns.
robolectric/robolectric#10428 (comment)
There was a problem hiding this comment.
@takahirom
I see they’ve likely “fixed” some of those issues, and if it works it’d be better since we’d not need to recreate the activity. Let me check if we cover all these cases I mentioned with screenshot tests. If not I’ll create a PR to cover them and we can see whether that works indeed as expected
There was a problem hiding this comment.
@takahirom
The tests for the corresponding cases are already there... and I can also confirm it does NOT fully work.
For this Preview:
@Preview(
name = "Preview width & height large",
widthDp = 2000,
heightDp = 1000,
)it generates a square image... although it should not (width and height are different). In Android Studio it displays as expected.
Was it merged although the test failed? I can confirm it displays as expected in 1.45.1 i.e. rectangle with the right dimensions instead of a square.
Maybe sth worth reporting in Robolectric?
There was a problem hiding this comment.
@sergio-sastre Thank you for finding this out!
Here is the diff in this pull request:
The diff was visible in #699 comment, but I'm not sure why it's missing here. I'll investigate and figure out the next steps.
There was a problem hiding this comment.
Okay, I understand what happened now. The screenshot name contains an ampersand (&), which is causing the CI to skip the test. I'll fix the CI configuration first.
No description provided.