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

LinuxMain template #125

Merged
merged 1 commit into from Jan 18, 2017
Merged

LinuxMain template #125

merged 1 commit into from Jan 18, 2017

Conversation

ilyapuchka
Copy link
Collaborator

Resolves #118

@krzysztofzablocki
Copy link
Owner

Let's ping @vknabel and @swizzler, since they know this usecase better, looks good to me.

@vknabel
Copy link
Contributor

vknabel commented Jan 16, 2017

@ilyapuchka @krzysztofzablocki Sorry for the late response. In my case the stencil template got parsed wrong and therefore an error has been thrown. Maybe the Stencil versions declared by SwiftPM and CocoaPods got out of sync?

$ sourcery --version
0.5.3
$ sourcery Tests Templates/LinuxMain.swift.stencil Tests/LinuxMain.swift
Scanning sources...
Found 3 types.
Loading templates...
Loaded 1 templates.
Generating code...
2017-01-16 12:11:38.066 sourcery[37327:9309308] *** Terminating app due to uncaught exception 'NSUnknownKeyException', reason: '[<sourcery.Method
0x7fa0034144f0> valueForUndefinedKey:]: this class is not key value coding-compliant for the key shortName|hasPrefix:"stest".'

But everything else looks promising.

@ilyapuchka
Copy link
Collaborator Author

ilyapuchka commented Jan 16, 2017

@vknabel I think that may be an issue, it should point to stencilproject/Stencil@99efba5

@krzysztofzablocki
Copy link
Owner

that's my bad, I'll update it in my next pr

@vknabel
Copy link
Contributor

vknabel commented Jan 16, 2017

@ilyapuchka I have released a new tag. (Sourcery still needs to use my fork for Stencil)
Parsing the template now works as expected, but I'll need to test it on linux first,

@vknabel
Copy link
Contributor

vknabel commented Jan 16, 2017

@ilyapuchka Works great! Shall be add some warnings when the methods/classes are not public? This would be an compile error on Linux.

@ilyapuchka
Copy link
Collaborator Author

@vknabel we ignore everything private at this moment. With #45 it will change of course. If there are private methods accessible then it's most likely a bug.

@vknabel
Copy link
Contributor

vknabel commented Jan 16, 2017

@ilyapuchka by default they are internal, but the LinuxMain.swift is not inside the test target.

@krzysztofzablocki
Copy link
Owner

We should add access level filter if we don't have it, it will come handy

@ilyapuchka
Copy link
Collaborator Author

@vknabel That explains why I was using @testable to import test module =) So I guess there are different options. And yes, filters will be handy.

@ilyapuchka
Copy link
Collaborator Author

@vknabel actually I'm not sure about warnings, feels like it's better just to make generated code not to compile if user forgot to make test method public. If he does not want to include it in test he can always adjust template to use annotations for methods or to prefix those methods names with _ or what ever. But that's more like a general best practice for templates that we should recommend or not. @krzysztofzablocki what do you think?

@krzysztofzablocki
Copy link
Owner

why don't we just filter to only support public in the template? otherwise I'd agree with @ilyapuchka, if user misuses the template its better to make it not compile so they fix their setup

@ilyapuchka
Copy link
Collaborator Author

@krzysztofzablocki we can, but I guess the whole purpose of such template is to never have any tests missing because you forgot to add it in XCTMain. So now you don't have to add methods manually but you still can forget to make them public. If we filter methods in template nothing will remind user about missing test. So one way is not to filter and make it fail to compile, another one is to filter but when filtering leave a comment in code.

@krzysztofzablocki
Copy link
Owner

I'd go with fail to compile, Sourcery driving goal is to limit human errors

@vknabel
Copy link
Contributor

vknabel commented Jan 17, 2017

@krzysztofzablocki @ilyapuchka Sorry for the confusion. It's good that the build breaks when one test is not public. At least it breaks when testing on Linux. Initially I thought about additionally marking non-public tests with a comment ("testSomething", testSomething) // You need to mark this declaration as public, but that would mean that warnings would eventually be committed, which is bad.

Instead I would just mention in the <details>, that the tests need to be public and hope that everyone reads it.

But this template works great. 👍

@ilyapuchka ilyapuchka merged commit 25b1c50 into krzysztofzablocki:master Jan 18, 2017
@ilyapuchka ilyapuchka deleted the linux-main branch January 18, 2017 11:29
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.

None yet

3 participants