goose icon indicating copy to clipboard operation
goose copied to clipboard

Move parser logic to internal package and add more tests

Open mfridman opened this issue 3 years ago • 2 comments

This PR moves the SQL parsing logic internally, mainly as a convenience to get it out of the main goose package. It also adds additional tests that make it easier to debug. Some of the previous output was in the form of:

t.Errorf("tt[%v] incorrect number of down stmts. got %v (%+v), want %v", i, len(stmts), stmts, test.down)

And this made it extremely difficult to reason why the output was different between got vs want. This PR adds golden files to capture the expected statements the parser must produce. If it does not, it writes a .FAIL file one can easily inspect during local development (see screenshot below). In CI this prints got and want as before.


Fix #198, #226 and another minor bug I found with a dangling comment while adding tests (yay tests).

In the screenshot, there are two issues.

  1. Comments that are ^-- get stripped, this is #198, #226
  2. There is a comment in the form of -- +goose StatementEnd at the very end of the statement. This doesn't affect the SQL statement itself, but we should fix that up.

CleanShot 2022-07-13 at 21 06 46@2x

mfridman avatar Jul 14 '22 01:07 mfridman

Outstanding, fix parser so it can continue to work with empty SQL migration files:

This is valid

-- +goose Up
-- This is just a comment

and so is:


-- comment
-- +goose Up

-- comment
-- +goose Down

EDIT: this is fixed and a test has been added

mfridman avatar Jul 14 '22 02:07 mfridman

Add a failing test for another edge case. Comments outside statements should not be included in the final statement.

mfridman avatar Jul 14 '22 13:07 mfridman

Closing this in favor of #444, #445 and subsequent parser fixes.

mfridman avatar Jan 20 '23 14:01 mfridman