rubenv / sql-migrate

SQL schema migration tool for Go.
MIT License
3.23k stars 280 forks source link

Update integration tests #62

Closed nicklaw5 closed 7 years ago

nicklaw5 commented 7 years ago
rubenv commented 7 years ago

What's wrong with the flag test?

nicklaw5 commented 7 years ago

I'm not entirely sure. The grep seems to not pick up the stdout. I ended up tweaking it a bit but in the end I couldn't get it to pass.

rubenv commented 7 years ago

As a result of adding the new Go versions?

It has always worked, so there must be something going on here.

rubenv commented 7 years ago

Found it:

$ sql-migrate status -config=test-integration/dbconfig.yml -env mysql_noflag
dial tcp 127.0.0.1:3306: getsockopt: connection refused

So the config is wrong. Trying a fix.

rubenv commented 7 years ago

On Travis:

Error 1045: Access denied for user ''@'localhost' (using password: NO)

How did this work before?

rubenv commented 7 years ago

Ugh, ignore that last comment.

nicklaw5 commented 7 years ago

From my testing, it wasn't working from Go v1.6+. I just assumed you left testing at v1.5 and below because you were aware of the issue, as I saw that you bumped it to v1.6 once but then bumped it back after the tests failed. see here

rubenv commented 7 years ago

Yeah, I remember now, didn't have time to look into it then.

Turns out it gives this:

sql: Scan error on column index 0: unsupported Scan, storing driver.Value type []uint8 into type *time.Time

As a general rule I don't remove tests unless there's a very good reason to do so (they're there to prevent regressions).

This one shouldn't be removed, we should fix it (otherwise users will get a very cryptic error message).

Question is mainly: why do we connect to the DB before checking the presence of that flag?

nicklaw5 commented 7 years ago

This seems related

rubenv commented 7 years ago

Just added this to work around it: https://github.com/rubenv/sql-migrate/commit/f5b76a4acc3bb13f1e61a7721dd9cb4b6cc54304

nicklaw5 commented 7 years ago

Sweet, I'll adjust my PR.

rubenv commented 7 years ago

No worries, I cherry-picked the other two.

It's merged, thanks a lot!