WIP: Created SqlResourceMigration to read sql from a resource file

[sdether]

Uses the Migration id to look for $id.sql in the resources of the provided class (could not find a good way to divine this class automatically).

Wasn't able to figure out how to run the current tests from either sbt or IntelliJ, so have not added tests go along and could use feedback on whether this is the right direction for inclusion in forklift.

I did test it against a local forklift test app and it seems to work as advertised

[CLAassistant]

All committers have signed the CLA.

[sdether]

Any suggestions on what I might have to do to get test running locally. Both test and test:test (as i saw CI doing) just go into an infinite build loop on my machine. Thanks

[lastland]

There are a few things:

1. You need to do sbt publishLocal before running tests (you have probably found that in circle.yml: https://github.com/lastland/scala-forklift/blob/develop/circle.yml#L19).
2. Running unit tests require setting up several databases on your local machine. You may want to ignore those cases, and just do testOnly com.liyaos.forklift.slick.tests.subprojects.* on your machine, and let CI tests the rest.

[sdether]

I already do publishLocal (it's how i test against my test project).

Running testOnly com.liyaos.forklift.slick.tests.subprojects.* starts with with below (i.e. no tests found) and then goes back into a compilation loop. Something seems to be wrong in my local setup

IJ]> testOnly com.liyaos.forklift.slick.tests.subprojects.*
[warn] Credentials file /Users/arne/.ivy2/.credentials does not exist
[warn] Credentials file /Users/arne/.ivy2/.credentials does not exist
[warn] Credentials file /Users/arne/.ivy2/.credentials does not exist
[warn] Credentials file /Users/arne/.ivy2/.credentials does not exist
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[info] No tests to run for scala-forklift-core/test:testOnly
[info] No tests to run for scala-forklift-plain/test:testOnly
[info] No tests to run for scala-forklift-git-tools/test:testOnly
[info] No tests to run for scala-forklift/test:testOnly

[lastland]

The messages you got are completely expected since the tests can only be found in subproject scala-forklift-slick. We shall see what happens on CI to see whether there is something wrong with the testing framework.

[sdether]

Looks CI still builds with 2.11 (or at least also 2.11) and so Source.fromResource is invalid.. Should I revert back or is the intention of this only being build with 2.12?

[lastland]

Right... Sorry, I forgot that. It would be good if 2.11 would still be supported...

What do you think if we revert back using getResourceAsStream. But instead of taking an extra argument in the constructor, we do something like this.getClass inside SqlResourceMigration?

[sdether]

That was what I was struggling with originally and why included the clazz argument. If i use this.getClass, it looks in the resources of forklift, not the project that has the main in it. Apparently Source.fromResource accomplishes this by using the ClassLoader from the current thread, so I will try to re-create that behavior using Source.fromInputStream and getResourceAsStream so that I can avoid the class argument.

[lastland]

Just checked the CI report. I think all the errors are in tests.

In particular,

1. The forklift version in example should also be changed to 0.3.2-SNAPSHOT (I should have an automatic procedure which does that in the testing script!).
2. This line in testing needs some changes because an existing 4.sql is now introduced (my code there is too ad-hoc): https://github.com/lastland/scala-forklift/blob/develop/migrations/slick/src/test/scala/SubProjectsTests/CommandsTests.scala#L88
3. This may not break anything, but I just remember this line may also need to be changed to actually test the new feature: https://github.com/lastland/scala-forklift/blob/develop/migrations/slick/src/test/scala/SubProjectsTests/MigrationDatabaseTests.scala#L37

[lastland]

I think the changes you made should be good (thanks a lot!). I can merge it and change the tests from my side (I can do that tonight), unless you want to do some hackings. 🙂

[sdether]

Go ahead and merge and make the changes, since I can't seem to figure out where I'm going wrong with CI. Thanks!

[lastland]

The PR has been merged. And the testing has been fixed (https://github.com/lastland/scala-forklift/commit/2c257585e9cad75ff518a8facfc34dbcbe7c4c34). I will publish the 0.3.2-SNAPSHOT version.

Thanks a lot for your contribution!