Skip to content

Fix(espresso): Fix #2349 #2359

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix(espresso): Fix #2349 #2359

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions espresso/core/java/androidx/test/espresso/base/ConfigurationSynchronizationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static java.util.Collections.unmodifiableList;

import android.app.Application;
import android.app.Activity;
import android.content.Context;
import android.os.Build;
Expand Down Expand Up @@ -49,6 +50,11 @@ public static void waitForConfigurationChangesOnActivity(
if (Build.VERSION.SDK_INT >= 24 && currentActivity.isInMultiWindowMode()) {
return;
}
// If the application is running activities in different processes, activities that aren't
// on the main process may have a different orientation
if (Build.VERSION.SDK_INT >= 28 && !currentActivity.getApplicationInfo().processName.equals(Application.getProcessName())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused how this statement could ever be true. I'm not familiar with this class but I don't see how it could be called with an Activity from a different process.

Do you have a test that can both repro the problem you are seeing and verify this fixes it?

Copy link
Author

Choose a reason for hiding this comment

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

@brettchabot Yes, as mentioned in #2349, there is this repository: https://github.com/Mathias-Boulay/expresso-issue

Branch main: the issue will trigger
Branch fix-included (newly added for your convenience): the issue won't trigger, as it uses the fixed library.

You should be able to use Android Studio Meerkat for both branches to run the only test in the project

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK I think I understand how this fix could work. IIUC currentActivity should always be running in the current process, but it could be possible that the Application's default process is different.

However, doesn't this change run the risk of introducing flakiness? With this change it looks like waitForConfigurationChangesOnActivity will always return immediately if the currentActivity is running in a secondary process.

I suppose it is no different than when the activity is in multi window mode....

I'll import this change internally and run a few more tests. Sorry for the delay in getting this reviewed

Copy link
Collaborator

Choose a reason for hiding this comment

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

currentActivity.getApplicationInfo().processName can be null in Robolectric environments. Consider using Objects.equals

return;
}

int applicationOrientation = appContext.getResources().getConfiguration().orientation;
if (applicationOrientation != currentActivity.getResources().getConfiguration().orientation) {
Expand Down