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

Added Linux library Unit testing to boilerplate #20

Merged
merged 3 commits into from Nov 25, 2016
Merged

Added Linux library Unit testing to boilerplate #20

merged 3 commits into from Nov 25, 2016

Conversation

idcrook
Copy link
Contributor

@idcrook idcrook commented Nov 25, 2016

  • Did NOT add a question about Linux support. Instead, added to
    Templates/ code that will still work unchanged under macOS, *OS. This
    is related to other details that I will describe.

  • do not see a conventional way to conditionally include snippets into
    a Template/ output file, so instead I added a conditional extension
    for Linux only. Snippets ability would be useful in many ways:
    include example unit testing code and an example object (struct,
    e.g.) in the Sources/ if, and only if, an optional Boolean such as
    "Linux Unit testing support" was added to user prompt.

  • tested under macOS. running testStructure.py passed.

  • compilation under Linux will fail, due to testExample not being
    defined (see above about example unit testing code)

 - Did NOT add a question about Linux support.  Instead, added to
   Templates/ code that will still work unchanged under macOS, *OS. This
   is related to other details that I will describe.

 - do not see a conventional way to conditionally include snippets into
   a Template/ output file, so instead I added a conditional extension
   for Linux only. Snippets ability would be useful in many ways:
   include example unit testing code and an example object (struct,
   e.g.) in the Sources/ if, and only if, an optional Boolean such as
   "Linux Unit testing support" was added to user prompt.

 - tested under macOS. running `testStructure.py` passed.

 - compilation under Linux will fail, due to `testExample` not being
   defined (see above about example unit testing code)
@idcrook
Copy link
Contributor Author

idcrook commented Nov 25, 2016

This is for #17 but it is NOT the implementation as outlined there. See above for details.

static var allTests : [(String, ({PROJECT}Tests) -> () throws -> Void)] {
// FIXME: Should return a list of test case (name, function) tuples
return [
("testExample", testExample),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we just create a simple test method that does nothing, so that it compiles out of the box?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A no-op? I like that idea. getting ready to send commit that does this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly! 👍

Added test method that does nothing, so that it compiles out of the
box (under Linux)
@testable import {PROJECT}Tests

XCTMain([
testCase({PROJECT}Tests.allTests),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎨 Indenation is a bit off here, but I can always fix that post-merge 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops. this was me using Emacs to insert code instead of Xcode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, is there a linter that you know of that can spot these kind of things?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SwiftLint seems to be quite popular in the community. Haven't tried it myself yet though (it's still on my todo list).

#if os(Linux)
extension {PROJECT}Tests {
static var allTests : [(String, ({PROJECT}Tests) -> () throws -> Void)] {
// FIXME: Should return a list of test case (name, function) tuples
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this now, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. That is actually needed by the way that swift test and the XCT* Framework works under Linux, at least as of today. That is also where/why the LinuxMain.swift comes in...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry - I meant that we can remove the FIXME comment. We still need the array.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright - just a thought! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK. Sure. I will bundle the changes... (our comments crossed paths, so I deleted my earlier one)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, cool!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the //FIXME in the templated code, I guess it's an interesting point on whether to leave it in... since this list is separately "hand" maintained for Linux testing?

I deleted it in this PR though.

Copy link
Contributor Author

@idcrook idcrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this manually on macOS. just noticed that it always clone from main repo. Will file an issue to add a command line switch to clone instead the local clone on the filesystem. --local-clone or some such

@JohnSundell
Copy link
Owner

@idcrook Ah, that's a great idea! 🚀

@JohnSundell
Copy link
Owner

So I'll proceed with the merge then @idcrook? 😄

 - Deleted the commented FIXME in the Linux test list
 - Formatted `LinuxMain.swift` according to conventions
@JohnSundell
Copy link
Owner

Great stuff - thanks for doing this @idcrook! 🎉

@JohnSundell JohnSundell merged commit 1bac999 into JohnSundell:master Nov 25, 2016
@idcrook
Copy link
Contributor Author

idcrook commented Nov 25, 2016

I added a commit for the remaining review items. Ready to merge

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

2 participants