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

Adding cycle selection on repeated shortcut press feature #1016

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexisgaziello
Copy link

  • CycleSelection is where most of the logic is stored. We use CGEvent instead7q of NSEvent.addGlobalMonitorForEvents because the latter doesn't allow taking over the event, which can lead to weird behavior. Note: right now this leads to a mix of NSEvent and CGEvent in the code which is not ideal.
  • Added tests for new feature
  • Split class MaccyUITests into BaseTest to re-use code

Other versions:

@alexisgaziello alexisgaziello force-pushed the feature/cycle-popup-v3 branch 3 times, most recently from d990141 to 1894244 Compare January 8, 2025 17:24
@alexisgaziello alexisgaziello marked this pull request as draft January 9, 2025 16:21
@alexisgaziello alexisgaziello marked this pull request as ready for review January 9, 2025 16:23
@alexisgaziello alexisgaziello force-pushed the feature/cycle-popup-v3 branch 3 times, most recently from 3747153 to 93760fe Compare January 11, 2025 18:57
@alexisgaziello
Copy link
Author

Sorry I just realized I didnt fix lints in the last revision. Lints are now fixed. Tests are still failing in CI, but they pass locally. Is the CI broken, or did I mess up some config?

- OpenShortcut.swift is where most of the logic is stored. We use CGEvent insteadq
 of NSEvent.addGlobalMonitorForEvents because the latter doesn't allow
 taking over the event, which can lead to weird behavior. Note: right now
 this leads to a mix of NSEvent and CGEvent in the code which is not ideal.
- Added tests for new feature
- Split class MaccyUITests into BaseTest to re-use code
@p0deje
Copy link
Owner

p0deje commented Jan 20, 2025

There are failures in master branch, so it's not your changes.

@alexisgaziello
Copy link
Author

I noticed a SwiftLint issue, but after spending more time on this, I believe I may have made a mistake elsewhere.

The failing tests are related to the shortcut functionality and might be permission-related. Here are the relevant permissions on my Mac:

sqlite> SELECT * FROM access WHERE client LIKE '%Maccy%';
service                   client                             client_type  auth_value  auth_reason  auth_version  csreq  policy_id  indirect_object_identifier_type  indirect_object_identifier  indirect_object_code_identity  flags  last_modified  pid  pid_version  boot_uuid  last_reminded
------------------------  ---------------------------------  -----------  ----------  -----------  ------------  -----  ---------  -------------------------------  --------------------------  -----------------------------  -----  -------------  ---  -----------  ---------  -------------
kTCCServicePostEvent      org.p0deje.MaccyUITests.xctrunner  0            2           4            2             ��                0                                UNUSED                                                     0      1736960607                       UNUSED     0
kTCCServiceAccessibility  org.p0deje.MaccyUITests.xctrunner  0            2           4            1             ��                0                                UNUSED                                                     0      1736960607                       UNUSED     0
kTCCServiceAccessibility  org.p0deje.Maccy                   0            2           4            1             ��                0                                UNUSED                                                     0      1736964343                       UNUSED     0

I'll try to fix the tests.

Additionally, I couldn't find the .bitrise.yaml file in the repo. I attempted to push a bitrise.yml file to see if Bitrise would pick it up, but it seems some configuration might be needed beforehand. Despite my tests, the configuration YAML doesn’t appear to change.

Would appreciate any guidance on both issues!

@p0deje
Copy link
Owner

p0deje commented Jan 20, 2025

Bitrise is configured manually via UI, which is why there is no configuration file. As I mentioned before, I think it's not worth splitting UI tests into 2 classes (the current has accessibility configured already), so if you join them back, it should be working on CI.

@alexisgaziello
Copy link
Author

If you think it's better to have all in a giant file I will join it back again, however I don't think that's the failure root cause. See the output; some MaccyUITests are also failing:

MaccyUITests
    ✓ testClear (14.371 seconds)
    ✓ testClearAll (13.758 seconds)
    ✓ testClearDuringSearch (14.113 seconds)
    ✗ testCloseWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 0` for object "280BE2EB-5C9E-4F98-BB01-380F0E32A67E" Any".
    ✗ testCloseWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 0` for object "A6E76341-0168-4B72-9E66-128420C4FB1A" Any".
    ✗ testCloseWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 0` for object "1EE92D93-8B17-40DA-8317-6B5C1DEC5092" Any".
    ✓ testControlJ (7.096 seconds)
    ✓ testControlK (7.072 seconds)
    ✓ testCopyFile (11.860 seconds)
    ✓ testCopyHTML (12.205 seconds)
    ✓ testCopyImage (10.457 seconds)
    ✓ testCopyWithClick (7.399 seconds)
    ✓ testCopyWithCommandShortcut (7.068 seconds)
    ✓ testCopyWithEnter (7.320 seconds)
    ✓ testCreatesNewCopyOnEnterWhenSearchResultsAreEmpty (7.924 seconds)
    ✓ testDeleteEntry (9.671 seconds)
    ✓ testDeleteEntryDuringSearch (12.038 seconds)
    ✓ testDisablesOnlyForNextCopyOnOptionShiftClickingMenubarIcon (11.468 seconds)
    ✓ testDisablesOnOptionClickingMenubarIcon (11.896 seconds)
    ✓ testDownArrow (7.157 seconds)
    ✓ testNewCopyIsAdded (11.047 seconds)
    ✓ testPasteToSearch (8.776 seconds)
    ✓ testPin (10.798 seconds)
    ✓ testPinDuringSearch (10.787 seconds)
    ✗ testPopupWithHotkey, failed - Maccy did not pop up
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "8BF17B0A-6069-49FC-8013-D1C9494E3BF8" Any".
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "6B3331BB-01B6-48A0-A04B-2C5D1EC31AB2" Any".
    ✗ testPopupWithHotkey, failed - Maccy did not pop up
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "42F04B16-38C2-47C4-B805-15AFCB75D765" Any".
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "8A21FF09-53B6-43E7-A0C4-C09BDA090895" Any".
    ✗ testPopupWithHotkey, failed - Maccy did not pop up
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "5F51660F-3B1E-41AB-B034-E36D831EEEB8" Any".
    ✗ testPopupWithHotkey, Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "Expect predicate `exists == 1` for object "D646B6B8-A51F-46A9-8894-860AB15FCB41" Any".
    ✓ testPopupWithMenubar (8.073 seconds)
(...)

Would you be against having the bitrise configuration in the repo?

@p0deje
Copy link
Owner

p0deje commented Jan 22, 2025

If you think it's better to have all in a giant file I will join it back again

Yes, please let's do it.

Would you be against having the bitrise configuration in the repo?

I am not against it, but it was a bit harder to iterate over the config file rather than just changing things in UI.

@alexisgaziello
Copy link
Author

I am not against it, but it was a bit harder to iterate over the config file rather than just changing things in UI.

Iterate would just be pushing the new yaml right? Seems simpler than editing code in 2 locations (UI & Github). Other benefits include having visibility over the CI from the repo, and allowing different configurations for different branches; for example right now I only want to play around and edit the config for this PR, not the whole repo.


In any case I'm fine editing the UI, but I'll probably need some sort of permissions right? Could you help me understand how to edit the yaml?

@p0deje
Copy link
Owner

p0deje commented Jan 23, 2025

@alexisgaziello You wouldn't need to change bitrise config, I've already fixed CI failures in master so as long as you keep new tests inside MaccyUITests target, they should work fine.

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.

2 participants