Skip to content

Commit

Permalink
Add basic tests for MacVim
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ychin
ychin committed Oct 26, 2023
1 parent 1f30462 commit 9fe55b0
Show file tree
Hide file tree
Showing 12 changed files with 1,002 additions and 98 deletions.
16 changes: 13 additions & 3 deletions .github/workflows/ci-macvim.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
157 changes: 72 additions & 85 deletions src/MacVim/MMAppController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -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
{
Expand All @@ -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:
Expand All @@ -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"];
Expand Down
4 changes: 4 additions & 0 deletions src/MacVim/MMBackend.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions src/MacVim/MacVim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 9fe55b0

Please sign in to comment.