Add integration-tests to waterline-sequel

[dmarcelino]

These tests have the same purpose as those added in balderdashy/waterline-adapter-tests#36 (click for details). These test sails-mysql and sails-postgresql edge versions against waterline-sequel edge version. They differ from the tests already existing in sails-mysql and sails-postgresql in the sense that those only test against waterline-sequel stable. Thus these tests are useful for waterline-sequel developers and to assess their PRs.

First paragraph breakdown in table format:

project tests	waterline-sequel version	sails-mysql / sails-postgresql version
sails-mysql	stable	edge
sails-postgresql	stable	edge
waterline-sequel	edge	edge

Example build: https://travis-ci.org/balderdashy/waterline-sequel/jobs/56210960#L407

[dmarcelino]

@tjwebb, @devinivy, @particlebanana, please take a look at this. TL;DR: similar concept as the waterline-adapter-tests but to help waterline-sequel developers to be safe their changes don't break sails-mysql and sails-postgresql.

[dmarcelino]

@tjwebb, @devinivy, @particlebanana, can we merge this so it becomes easier to assess other PRs? e.g. check if other PRs break the sails-mysql/sails-postgresql.

[devinivy]

My only concern is that this will break the badge on this repo, currently for reasons unrelated to waterline-sequel. Is this a chicken/egg problem, or can we resolve those bugs first? Should the badge for waterline-sequel be dependent on edge versions of other repos, or should this merely be an available tool for developers to test their PRs?

Or do I understand this a bit incorrectly?

[dmarcelino]

@devinivy, the findOrCreateEach bug is responsible for breaking 4 tests in sails-mysql and another 4 in sails-postgresql. Once waterline core is released those will go away. Apart from that only 1 broken test will remain in sails-mysql:

Association Interface Belongs To Association .find() should return customer when the populate criteria is added:
     Uncaught AssertionError: false == true

I agree it's the chicken and the egg problem and regarding that I believe a broken badge is a great motivation for people to fix things! :D

If we keep hiding broken tests as we have so far issues will remain and they will bite us when we less expect.

PS: the integrated tests only cover official/balderdashy projects, so it's in our grasp to fix all of these

[devinivy]

I understand that! I just wonder if the badges should be separate. I think waterline-sequel should pass if it works according to its API. Otherwise when something does break, all the badges will go red together, and it will be hard to pinpoint exactly which module is failing. I do, however love that these tests are available to be easily run by developers evaluating PRs.

[devinivy]

To be clear: I don't think this is quite the same as running waterline against sails-memory. That is a great vanilla use-case for waterline having few dependencies. It's almost like testing with a stub adapter.

[dmarcelino]

I agree with the separation, the annoying thing with Travis (in the free version at least) is that we would need a separate github project for that, something like a waterline-sequel-tests repo just for running the integration tests (a bit like we did with waterline <-> waterline-adapter-tests integration tests). And the main problem with it is we don't see the results in the waterline-sequel PRs as they wouldn't be chained. In other words there would be the risk of someone breaking sails-postgresql or sails-mysql and nobody noticing until a waterline-sequel release.

[devinivy]

I still think that in this case, the onus is on whomever publishes edge-->stable to run tests against official SQL sails adapters. It could be with an easy-to-use command that this repo provides (which this PR includes). The only difference is that the badge on this repo reflects waterline-sequel specific tests. Frankly, someone could misuse waterline-sequel in sails-mysql– that doesn't mean that waterline-sequel is broken. It's just a matter of what Travis tests on this repo. I really do think it's cleaner that way and will cause us less trouble.

[devinivy]

It may mean that this repo needs more, or better tests. This sort of change basically means that we don't trust the tests in this repo, which ought to reflect the promise that this piece of software makes to other pieces of software. I think we need a third voice here to figure out the best route. @particlebanana @tjwebb ?

EDIT: @dmarcelino what about the option of Travis running the tests for reference, but not allowing them to cause a failure. That way we could view the Travis builds while evaluating a PR.

[dmarcelino]

The thing is... I don't think everyone who develops waterline-sequel have both MySQL an Postgresql installed in their machines. Even if they have, depending on their machines setups the tests may pass for them but break for others, as it wasn't a standardised, from scratch build.

This sort of change basically means that we don't trust the tests in this repo, which ought to reflect the promise that this piece of software makes to other pieces of software.

I disagree, if the build is broken we should fix it. We shouldn't be releasing waterline-sequel changes that we can't objectively confirm are working with sails-mysql and sails-postgresql (AKA Travis builds as opposed to "it works on my machine").

Last, I believe waterline-sequel is too tethered to sails-mysql and sails-postgresql for us not to constantly test against them. I agree my implementation is not perfect but without a tool like Jenkins or similar it's hard to do these sort of checks in a cleaner way (and I'm completely open to suggestions).

[dmarcelino]

Alright I have an idea... we can try setting up https://circleci.com in addition to Travis. Travis would run the unit tests as we are used to and circleci would run the integration tests. I never worked with circleci so I'm not sure how well it integrates with github but this solution would give us separation of concerns. What do you guys think?

[devinivy]

I haven't worked with that either. Any thoughts about the idea I edited-in on https://github.com/balderdashy/waterline-sequel/pull/32#issuecomment-88923784 ?

[dmarcelino]

Sorry, didn't notice it...

what about the option of Travis running the tests for reference, but not allowing them to cause a failure. That way we could view the Travis builds while evaluating a PR.

It's easy to do, that's the current situation for sails-mongo and sails-mysql builds. The problem I see with it is most people will ignore the integration tests. Sails-mongo (build) and Sails-MySQL (build) have broken tests for some time and I don't think anyone noticed because we see the :white_check_mark: and think everything is fine. In other words, we already have a bad track record with that kind of scenario... :disappointed:

[devinivy]

I guess I'll just say that I think it's atypical to tie passing-ness of modules together, and it comes with some problems of its own. We need to be able to tell which part of the whole system is failing. If all badges go red, it becomes very hard to tell.

If waterline-sequel passes its tests but one of the SQL adapters is failing, there's possibly a larger issue, and it truly might just be the adapter catering to a bug in a dependency (like waterline-sequel). The most we can do, as I see it, is make the integration tests available and perhaps make using them a part of the publish process.

Again, I might be up my own arse, and I'd love some participation from other interested parties.

[dmarcelino]

@devinivy, the build logs from travis are quite good for pinpointing where/when the issue happened. When I see a red badge I always click on it and check where it broke and trace it to a commit.

There is another nuisance with the whole module things... For instance, currently sails-mysql and sails-postgresql point to waterline-sequel 0.1.X (package.json) even though last released version of waterline-sequel is v0.2.0. This means that if you trigger a new build of sails-mysql/postgresql they won't tell you if waterline-sequel has any breaking changes. So, for a waterline-sequel developer (or a PR reviewer) there is no early warning mechanism to tell him if he screwed up.

[dmarcelino]

Alright, I've managed to set up Circle CI for integration tests for a test repo, you can see the results here:

I would need a repo owner (@particlebanana ?) to setup waterline-sequel in circleci.com and github integrations and that would allow the best of both worlds: separate badges for unit and integration builds!

PR #34 uses circleci for integration.