If there is a space in front of --migrate:down, the migration fails, silently

[nichochar]

I encountered a strange bug: I had a space in front of -- migrate:down, and the migration was getting applied, successfully, except that it wasn't being run (silently failing).

This was pretty bad and took a while for me to fix.

We should likely have stricter parsing of the syntax, if this can silently fail

[benjreinhart]

Hmm, interesting. What version of dbmate are you using?

It is supposed to fail in that case. The regular expression used to parse the -- migrate:up line should not match if the line does not start with --. In the event it does not match, it returns an error.

There are tests and I validated this regexp in a go playground.

Any chance you could paste from line 0 up to and including the -- migration:up block to see if I can reproduce?

[benjreinhart]

Ohh I just realized this is for the -- migrate:down block. The regular expression for down expects the same strictness, but since it's allowed to be omitted, it's just being ignored. I do agree this is an interesting case that likely needs more thought.

That being said, I'm confused on:

the migration was getting applied, successfully, except that it wasn't being run (silently failing).

How is it being applied but not run? Can you clarify that? For example, is it making the changes in the migrate:up block? What is silently failing?

[amacneil]

Given that the -- migrate:down block is optional, it's expected that not following the expected syntax would not trigger any errors. However, this issue report makes me think that at the very least we should match whitespace preceeding -- migrate:down directives, and a more drastic measure might be to make -- migrate:down mandatory. The failure case here is certainly pretty confusing (since we keep both up and down migrations in the same file a typo would result in running both up and down migrations likely resulting in a net zero change to schema).