Allow the “bash requesting screen access” popup in macOs latest #1482
Conversation
There was a problem hiding this comment.
Hi @anusreelakshmi934 , I know this PR is still in draft mode, I just wanted to give some early feedback.
- First off, this looks really good. It's awesome that you found a solution for this.
- Did you write the AppleScript code yourself or borrow from another source? If borrowed, it's important we reference where we got it from.
- To avoid repeating the new
AllowPopupTesttest method for each of the different test classes, it would be good to create a base class that each of these classes could extend which contains the newAllowPopupTesttest method. I see there are some variations in some of theAllowPopupTestmethods to bring the build file into focus. Can that code be reused and just go in the base class? Maybe there is a check that can be used to see if the project is Gradle before running those steps? - Can you look into the test failures to verify that they are not related to these changes?
Hi Trevor. I wrote this script with the help of some references which are : Also when I searched a little deeper today I came across this link - https://smartwatermelon.medium.com/automating-macos-security-dialogs-a-tale-of-yak-shaving-and-applescript-759300d6fba9. I tried modifying this for our use. So now we no longer need to click on popup to bring it to focus. This script handles it. So I have removed the code to open and bring the build file into focus. I have also moved this test into a base class which is extended by other classes. I have verified the test failures are not due to this changes. I checked the test report videos and could see that the popup is no longer present. I have verified that the tests succeeds by running them in. my fork - https://github.com/anusreelakshmi934/liberty-tools-intellij/actions/runs/19734581320. Verified that the failures in first attempt are not related to these changes. |
TrevCraw
left a comment
TrevCraw
left a comment
There was a problem hiding this comment.
@turkeylurkey and I reviewed. Mostly, the changes look good. We have a few comments/questions to address.
| import org.junit.jupiter.api.condition.OS; | ||
|
|
||
|
|
||
| public class MacOSAllowPopupTest { |
There was a problem hiding this comment.
Discussed with @turkeylurkey . We'd like to suggest renaming the class to "BaseOSUtilities". It seems strange to have our common test classes extending a Mac specific class. This class could then be expanded in the future if other setup becomes necessary for any OS.
There was a problem hiding this comment.
Thanks for the suggestion. I have modified the name.
| import static com.intellij.remoterobot.utils.RepeatUtilsKt.waitForIgnoringError; | ||
|
|
||
| public abstract class SingleModJakartaLSTestCommon { | ||
| @TestMethodOrder(MethodOrderer.OrderAnnotation.class) |
There was a problem hiding this comment.
Is it possible to add this annotation to the base class? Will that work the way we want it to? If so, we could avoid adding this line to all the tests extending the base class.
There was a problem hiding this comment.
Yes. I have added it to just base class and removed it from others. It works as expected.
| TestUtils.sleepAndIgnoreException(10); | ||
| // Execute AppleScript to click the "Allow" button | ||
| try { | ||
| String appleScript = |
There was a problem hiding this comment.
It would be good to cite the source that was used as reference for this "Apple script". You can mention that the appleScript is based off an example provided at https://smartwatermelon.medium.com/automating-macos-security-dialogs-a-tale-of-yak-shaving-and-applescript-759300d6fba9. It would also be good to point to official documentation, like the one you shared here: https://developer.apple.com/library/archive/documentation/LanguagesUtilities/Conceptual/MacAutomationScriptingGuide/AutomatetheUserInterface.html - just in case a developer in the future needs to make changes.
There was a problem hiding this comment.
I have added these references.
2 participants
Fixes #1451
The fix is to add a test that runs first in each class, ensuring the popup is brought into focus and the Allow button is clicked before any other tests run. Added the new AllowPopupTest test method in a base class that each of the classes extend.