Add basic tests for MacVim

Add a few tests for MacVim using XCTest. Previously MacVim had
essentially zero automatic tests. While we run the Vim tests in CI,
those mostly exercise Vim features, not MacVim. These new tests are
Objective C tests that are designed for MacVim-layer functionality
instead. Only adding a few tests (both unit tests and integration tests)
now to get started and set up a template for different types of tests
but more will be added later.

Since MacVim consists of a lot of macOS API calls, a lot of those
functionality are not easy to test automatically as they rely on
behaviors out of MacVim's control (e.g. full screen functionality). The
goal here is not to get 100% test coverage, but to catch areas prone to
have regressions, code with non-straightforward logic, functionality
that are annoying to manually test (e.g. key input), code blocks that
perform calculations and can be easily unit tested. Areas that would be
harder to add tests for include integration with macOS, e.g. full screen
or dual monitor, interacting with the windowing system, or things that
require screenshots in order to properly validate.

The goal here is to progressively add more tests as we touch different
parts of MacVim to help catch regressions. Old code that works and isn't
being touched will be lower priority as it's unlikely they will suddenly
break.

Author: ychin

.github/workflows/ci-macvim.yaml

@@ -87,6 +87,11 @@ jobs:
           # Later, we pass the --enable-sparkle_1 flag to configure to set the corresponding ifdef.
           ln -fhs Sparkle_1.framework src/MacVim/Sparkle.framework
 
+          # Sparkle shows a dialog asking if the user wants to check for updates on 2nd launch of
+          # MacVim. On Sparkle 1 this is annoyingly a modal dialog box and interferes with tests.
+          # Just disable it by pre-setting to not check for updates.
+          defaults write org.vim.MacVim SUEnableAutomaticChecks 0
+
       - name: Set up Xcode
         if: matrix.xcode != ''
         run: |
@@ -310,7 +315,12 @@ jobs:
             echo 'MacVim_xcode8.xcodeproj is outdated. Run "make -C src macvim-xcodeproj-compat" to re-generate it.'; false
           fi
 
-      - name: Build test binaries
+      - name: Test MacVim
+        timeout-minutes: 10
+        run: |
+          make ${MAKE_BUILD_ARGS} -C src macvim-tests
+
+      - name: Build Vim test binaries
         run: |
           # Build the unit test binaries first. With link-time-optimization they take some time to link. Running them
           # separately de-couples them from the timeout in tests, and allow us to build in parallel jobs (since tests
@@ -320,11 +330,11 @@ jobs:
           set -o verbose
           make ${MAKE_BUILD_ARGS} -j${NPROC} -C src unittesttargets
 
-      - name: Test
+      - name: Test Vim
         timeout-minutes: 20
         run: make ${MAKE_BUILD_ARGS} test
 
-      - name: Test GUI
+      - name: Test Vim (GUI)
         timeout-minutes: 20
         run: |
           make ${MAKE_BUILD_ARGS} -C src/testdir clean

src/MacVim/MMAppController.m

@@ -121,6 +121,7 @@ - (NSArray *)filterOpenFiles:(NSArray *)filenames
 - (void)handleXcodeModEvent:(NSAppleEventDescriptor *)event
                  replyEvent:(NSAppleEventDescriptor *)reply;
 #endif
++ (NSDictionary*)parseOpenURL:(NSURL*)url;
 - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event
                replyEvent:(NSAppleEventDescriptor *)reply;
 - (NSMutableDictionary *)extractArgumentsFromOdocEvent:
@@ -523,30 +524,7 @@ - (void)applicationDidFinishLaunching:(NSNotification *)notification
             // If the current version is larger, set that to be stored. Don't
             // want to do it otherwise to prevent testing older versions flipping
             // the stored version back to an old one.
-            NSArray<NSString*> *lastUsedVersionItems = [lastUsedVersion componentsSeparatedByString:@"."];
-            NSArray<NSString*> *currentVersionItems = [currentVersion componentsSeparatedByString:@"."];
-            // Compare two arrays lexographically. We just assume that version
-            // numbers are also X.Y.Z… with no "beta" etc texts.
-            BOOL currentVersionLarger = NO;
-            for (int i = 0; i < currentVersionItems.count || i < lastUsedVersionItems.count; i++) {
-                if (i >= currentVersionItems.count) {
-                    currentVersionLarger = NO;
-                    break;
-                }
-                if (i >= lastUsedVersionItems.count) {
-                    currentVersionLarger = YES;
-                    break;
-                }
-                if (currentVersionItems[i].integerValue > lastUsedVersionItems[i].integerValue) {
-                    currentVersionLarger = YES;
-                    break;
-                }
-                else if (currentVersionItems[i].integerValue < lastUsedVersionItems[i].integerValue) {
-                    currentVersionLarger = NO;
-                    break;
-                }
-            }
-
+            const BOOL currentVersionLarger = (compareSemanticVersions(lastUsedVersion, currentVersion) == 1);
             if (currentVersionLarger) {
                 [ud setValue:currentVersion forKey:MMLastUsedBundleVersionKey];
 
@@ -2130,6 +2108,73 @@ - (void)handleXcodeModEvent:(NSAppleEventDescriptor *)event
 }
 #endif
 
++ (NSDictionary*)parseOpenURL:(NSURL*)url
+{
+    NSMutableDictionary *dict = [NSMutableDictionary dictionary];
+
+    // Parse query ("url=file://...&line=14") into a dictionary
+    NSArray *queries = [[url query] componentsSeparatedByString:@"&"];
+    NSEnumerator *enumerator = [queries objectEnumerator];
+    NSString *param;
+    while ((param = [enumerator nextObject])) {
+        // query: <field>=<value>
+        NSArray *arr = [param componentsSeparatedByString:@"="];
+        if ([arr count] == 2) {
+            // parse field
+            NSString *f = [arr objectAtIndex:0];
+#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
+            f = [f stringByRemovingPercentEncoding];
+#else
+            f = [f stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
+#endif
+
+            // parse value
+            NSString *v = [arr objectAtIndex:1];
+
+            // We need to decode the parameters here because most URL
+            // parsers treat the query component as needing to be decoded
+            // instead of treating it as is. It does mean that a link to
+            // open file "/tmp/file name.txt" will be
+            // mvim://open?url=file:///tmp/file%2520name.txt to encode a
+            // URL of file:///tmp/file%20name.txt. This double-encoding is
+            // intentional to follow the URL spec.
+#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
+            v = [v stringByRemovingPercentEncoding];
+#else
+            v = [v stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
+#endif
+
+            if ([f isEqualToString:@"url"]) {
+                // Since the URL scheme uses a double-encoding due to a
+                // file:// URL encoded in another mvim: one, existing tools
+                // like iTerm2 could sometimes erroneously only do a single
+                // encode. To maximize compatiblity, we re-encode invalid
+                // characters if we detect them as they would not work
+                // later on when we pass this string to URLWithString.
+                //
+                // E.g. mvim://open?uri=file:///foo%20bar => "file:///foo bar"
+                // which is not a valid URL, so we re-encode it to
+                // file:///foo%20bar here. The only important case is to
+                // not touch the "%" character as it introduces ambiguity
+                // and the re-encoding is a nice compatibility feature, but
+                // the canonical form should be double-encoded, i.e.
+                // mvim://open?uri=file:///foo%2520bar
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_10
+                if (AVAILABLE_MAC_OS(10, 10)) {
+                    NSMutableCharacterSet *charSet = [NSMutableCharacterSet characterSetWithCharactersInString:@"%"];
+                    [charSet formUnionWithCharacterSet:NSCharacterSet.URLHostAllowedCharacterSet];
+                    [charSet formUnionWithCharacterSet:NSCharacterSet.URLPathAllowedCharacterSet];
+                    v = [v stringByAddingPercentEncodingWithAllowedCharacters:charSet];
+                }
+#endif
+            }
+
+            [dict setValue:v forKey:f];
+        }
+    }
+    return dict;
+}
+
 - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event
                replyEvent:(NSAppleEventDescriptor *)reply
 {
@@ -2138,7 +2183,7 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event
                                         stringValue]];
 
     // We try to be compatible with TextMate's URL scheme here, as documented
-    // at http://blog.macromates.com/2007/the-textmate-url-scheme/ . Currently,
+    // at https://macromates.com/blog/2007/the-textmate-url-scheme/ . Currently,
     // this means that:
     //
     // The format is: mvim://open?<arguments> where arguments can be:
@@ -2151,66 +2196,8 @@ - (void)handleGetURLEvent:(NSAppleEventDescriptor *)event
     // Example: mvim://open?url=file:///etc/profile&line=20
 
     if ([[url host] isEqualToString:@"open"]) {
-        NSMutableDictionary *dict = [NSMutableDictionary dictionary];
-
-        // Parse query ("url=file://...&line=14") into a dictionary
-        NSArray *queries = [[url query] componentsSeparatedByString:@"&"];
-        NSEnumerator *enumerator = [queries objectEnumerator];
-        NSString *param;
-        while ((param = [enumerator nextObject])) {
-            // query: <field>=<value>
-            NSArray *arr = [param componentsSeparatedByString:@"="];
-            if ([arr count] == 2) {
-                // parse field
-                NSString *f = [arr objectAtIndex:0];
-#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
-                f = [f stringByRemovingPercentEncoding];
-#else
-                f = [f stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
-#endif
-
-                // parse value
-                NSString *v = [arr objectAtIndex:1];
-
-                // We need to decode the parameters here because most URL
-                // parsers treat the query component as needing to be decoded
-                // instead of treating it as is. It does mean that a link to
-                // open file "/tmp/file name.txt" will be
-                // mvim://open?url=file:///tmp/file%2520name.txt to encode a
-                // URL of file:///tmp/file%20name.txt. This double-encoding is
-                // intentional to follow the URL spec.
-#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
-                v = [v stringByRemovingPercentEncoding];
-#else
-                v = [v stringByReplacingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
-#endif
-
-                if ([f isEqualToString:@"url"]) {
-                    // Since the URL scheme uses a double-encoding due to a
-                    // file:// URL encoded in another mvim: one, existing tools
-                    // like iTerm2 could sometimes erroneously only do a single
-                    // encode. To maximize compatiblity, we re-encode invalid
-                    // characters if we detect them as they would not work
-                    // later on when we pass this string to URLWithString.
-                    //
-                    // E.g. mvim://open?uri=file:///foo%20bar => "file:///foo bar"
-                    // which is not a valid URL, so we re-encode it to
-                    // file:///foo%20bar here. The only important case is to
-                    // not touch the "%" character as it introduces ambiguity
-                    // and the re-encoding is a nice compatibility feature, but
-                    // the canonical form should be double-encoded, i.e.
-                    // mvim://open?uri=file:///foo%2520bar
-#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
-                    NSMutableCharacterSet *charSet = [NSMutableCharacterSet characterSetWithCharactersInString:@"%"];
-                    [charSet formUnionWithCharacterSet:NSCharacterSet.URLHostAllowedCharacterSet];
-                    [charSet formUnionWithCharacterSet:NSCharacterSet.URLPathAllowedCharacterSet];
-                    v = [v stringByAddingPercentEncodingWithAllowedCharacters:charSet];
-#endif
-                }
-
-                [dict setValue:v forKey:f];
-            }
-        }
+        // Parse the URL and process it
+        NSDictionary *dict = [MMAppController parseOpenURL:url];
 
         // Actually open the file.
         NSString *file = [dict objectForKey:@"url"];

src/MacVim/MMBackend.m

@@ -2319,6 +2319,10 @@ - (void)handleInputEvent:(int)msgid data:(NSData *)data
         [self setImState:NO];
     } else if (BackingPropertiesChangedMsgID == msgid) {
         [self redrawScreen];
+    } else if (LoopBackMsgID == msgid) {
+        // This is a debug message used for confirming a message has been
+        // received and echoed back to caller for synchronization purpose.
+        [self queueMessage:msgid data:nil];
     } else {
         ASLogWarn(@"Unknown message received (msgid=%d)", msgid);
     }

src/MacVim/MacVim.h

@@ -344,6 +344,7 @@ extern const char * const MMVimMsgIDStrings[];
     MSG(EnableThinStrokesMsgID) \
     MSG(DisableThinStrokesMsgID) \
     MSG(ShowDefinitionMsgID) \
+    MSG(LoopBackMsgID) /* Simple message that Vim will reflect back to MacVim */ \
     MSG(LastMsgID) \
 
 enum {