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

Fix issues with map type #242

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Conversation

programmingkidx
Copy link
Contributor

In the function type_convertion.go:convertToGoValue() it makes a call to reflect.ValueOf() for the reflect.Map case. This should not be done because the value that ToGoMap() returns is already of type reflect.Value. Also it makes type assertions fail for map types.

This program was used to test these two patches:

// Description: Test the (NS)Error class

package main

import f "github.com/progrium/macdriver/macos/foundation"
import "fmt"
import "github.com/progrium/macdriver/objc"
import "reflect"
import "github.com/progrium/macdriver/macos/appkit"

func main() {
	
	// declare the variables
	var domain f.ErrorDomain = "My Domain"
	code := -99
	var userInfo map[f.ErrorUserInfoKey]objc.IObject
	userInfo = make(map[f.ErrorUserInfoKey]objc.IObject)
	userInfo["one"] = f.String_StringWithString("ONE")
	userInfo["two"] = f.String_StringWithString("TWO")
	userInfo["three"] = f.String_StringWithString("THREE")
	userInfo["window"] = appkit.NewWindowWithSize(640, 480)

	// create the (NS)Error object
	myError := f.Error_ErrorWithDomainCodeUserInfo(domain, code, userInfo)
	
	// print out error and check field values
	fmt.Println("Testing Error class\n")
	fmt.Println("error.LocalizedDescription():", myError.LocalizedDescription())
	fmt.Print("error.Code(): ", myError.Code())
	if myError.Code() == code {
		fmt.Println("...correct")
	} else {
		fmt.Println("...wrong")
	}
	
	fmt.Print("error.Domain(): ", myError.Domain())
	if myError.Domain() == domain {
		fmt.Println("...correct")
	} else {
		fmt.Println("...wrong")
	}
	
	fmt.Println("error.UserInfo() value:", myError.UserInfo())
	fmt.Println("error.UserInfo() type:", reflect.TypeOf(myError.UserInfo()))
	fmt.Println("\texpected type: map[foundation.ErrorUserInfoKey]objc.IObject)")

	fmt.Println("\nReturned userinfo values:")
	for k,v := range myError.UserInfo() {
		fmt.Printf("key: %s\tvalue: %s\n", k, v.Description())
	}
	
	fmt.Println("\nExpected userinfo values:")
	for k,v := range myError.UserInfo() {
		fmt.Printf("key: %s\tvalue: %s\n", k, v.Description())
	}
}

Before these patches this program would panic at the call to myError.UserInfo(). With the patches applied this program works correctly.

@progrium
Copy link
Owner

Sorry for delay, but would it be possible to get a simplified version of this test program in as an actual test for the package?

@programmingkidx
Copy link
Contributor Author

I made this patch to test my tests. To apply it "cd" to the root level of your local repo and run this command: "patch -p1 < NSError-tests.patch".

I had to comment out the Dictionary and Array tests because they cause the test system to fail. This prevents the NSError tests from running.

With all my pull requests applied all the tests run successfully.

Here is the output on my machine:
go test ./macos/foundation -v
=== RUN TestFoundationValid
--- PASS: TestFoundationValid (0.00s)
=== RUN TestFoundationError
=== RUN TestFoundationError/Empty_Map
=== RUN TestFoundationError/Map_with_items_in_it
=== RUN TestFoundationError/Check_domain_and_code_parameters
--- PASS: TestFoundationError (0.00s)
--- PASS: TestFoundationError/Empty_Map (0.00s)
--- PASS: TestFoundationError/Map_with_items_in_it (0.00s)
--- PASS: TestFoundationError/Check_domain_and_code_parameters (0.00s)
PASS
ok github.com/progrium/macdriver/macos/foundation 0.109s
NSError-tests.patch

@progrium
Copy link
Owner

Why not make this patch part of this PR?

@programmingkidx
Copy link
Contributor Author

Done.

@progrium progrium merged commit 4533128 into progrium:main Jan 17, 2024
3 checks passed
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