Proposal: Remove unit test dependency from MySQL

[codingllama]

Currently, unit tests require a local MySQL to be running and have a root user without a password. While the presence of MySQL is paramount for integration-like tests, unit tests shouldn't have such requirement. That precludes users from doing a simple go test ./... and forces a specific local configuration.

I've had a quick look at sqlite in memory as substitute for unit tests and it looks viable.

A course of action could be:

1. Make tests rely on GetTestDB instead of opening connections directly
2. Replace the DB with sqlite3/:memory:
3. ?
4. Profit

From what I've seen, there a few changes we'd have to make for it to work:

• Remove '#' comments from storage.sql
• Declare indexes separately from table definitions
• Remove SET GLOBAL sql_mode
• Find a way around enums

A simple solution would be to have separate-but-equivalent schemas. That's not entirely desirable due to duplication, but it does fix a whole category of issues. Another venue is to "reinterpret" the script at GetTestDB and remove / change the problematic lines.

There are a few other options, such as writing our own fake storage implementation, but that doesn't solve the issue of storage layer tests.

WDYT?

[gdbelvin]

I was just working on something like this, but abandoned it for the moment.

In addition to the issues you've identified, I also identified the following:

• No support for SQL ENUMS
• Unique Indexes need to be declared outside tables

This would make it much easier to test Trillian with other projects. +1

For what it's worth we could supply a command line flag so the travis tests could still run using proper MySQL to make sure we haven't missed anything.

[codingllama]

Yep, I was expecting you to pitch along. :)
@pphaneuf may be interested as well, as we were talking about this earlier today.

I have an experimental branch with a bunch of similar changes to yours, although it's nowhere close to a complete solution.

I like the idea of a flag for Travis. We could run unit tests using both in-memory and MySQL to be extra safe. There's also less reason not to do this if we have that.

[Martin2112]

I did have a fake storage implementation back at the start of the project but it was too much work to maintain with things in flux.

I'd like to defer this until we've settled the schema a bit more. Note that there are some more issues with parameter ordering that might not affect SQLite but do affect sql variants with positional parameters like Postgres.

[Martin2112]

Oh and what's the proposal for unit testing the MySQL code without MySQL? :-)

[codingllama]

Re storage fakes: yeah, it can get pretty onerous early on. I did it before and, while it's handy, it also brings significant maintenance costs.

Re testing MySQL: integration tests would still require MySQL; we're aiming for the go test experience to be free of external setup, not everything.

There's also the idea of setting a DB flag for tests: by default we go with an in-memory storage, but if the flag is set to mysql then we use it. The same mechanism can be used to make Travis do 2 go test passes: one with in-memory and one with MySQL. It could also be used to test with MySQL locally.

If it all works as above, then we can do it without losing any functionality on our workflow, which would be pretty sweet. :)

How does that sound to you?

[Martin2112]

I think it sounds workable, it's just a matter of picking the best timing to do it. We can maybe drop the strict mode thing as I'm pretty sure it's now the default. But from experimenting with Postgres variants we probably need hooks to run when the database is opened or a transaction is created that implementations can use. Again, it's unclear how much of this work needs to be done now so if SQLlite is otherwise sufficiently like MySQL to share the code minus a few tweaks that might be enough for the moment. Martin

…

On 28 March 2017 at 10:50, Alan Parra ***@***.***> wrote: Re storage fakes: yeah, it can get pretty onerous early on. I did it before and, while it's handy, it also brings significant maintenance costs. Re testing MySQL: integration tests would still require MySQL; we're aiming for the go test experience to be free of external setup, not *everything*. There's also the idea of setting a DB flag for tests: by default we go with an in-memory storage, but if the flag is set to mysql then we use it. The same mechanism can be used to make Travis do 2 go test passes: one with in-memory and one with MySQL. It could also be used to test with MySQL locally. If it all works as above, then we can do it without losing any functionality on our workflow, which would be pretty sweet. :) How does that sound to you? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#456 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMv2T1EDwNIbclcSxEjECripZp78Yc6Nks5rqNftgaJpZM4MjyWm> .

[Martin2112]

Provisionally putting this in M9 to be done when things have settled down with the schema etc.