-
Notifications
You must be signed in to change notification settings - Fork 8
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
[wip] Load type infos correctly #11
base: main
Are you sure you want to change the base?
Conversation
@gtramontina Do you have thoughts about the disabled tests? |
Thanks for putting in the hard work, @blizzy78! 🙏
It seems that the Having the mutation tests running is a testment of the fix, although I'd like to have some level of coverage on I'm happy to move forward with these tests skipped for now, but I'll likely come back to them soon. |
if idx < 0 { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this ever happen? I'm thinking that if it ever happens, it means that something has actually gone wrong pretty badly, no? I'd consider blowing up with a panic here. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sounds good to me.
I didn't know package |
This PR changes the loading of type infos in
GoSourceFile.Incubate
such that mutation tests should no longer fail, a bug that had been introduced in #5. With this PR, all tests and mutation tests seem to run fine for me.I did have to explicitly disable
ooze_test.TestOoze_nothing_to_test
andooze_test.TestOoze_with_mutations
. These tests do not use regular Go source files on disk, but rather in-memory source "files". Perhaps these should be changed to regular files.This PR is still a work in progress because of those disabled tests.
I have removed
go:build testdata
flags from all Go source files intestdata
folders. For one, they don't seem to be necessary at all, and two, they prevent this PR from working.This PR supersedes #10.