Compile-test some tests

[hasezoey]

This PR is build on #112 (merged, rebased)

This PR changes some generation tests to also be compile-tested, which currently include:

• simple_table_pg
• simple_table_sqlite
• simple_table_mysql
• single_model_file
• custom_model_and_schema_path

this required the following changes:

• split simple_table out to their own connections, because not all types are compatible everywhere (ex Unsigned is a mysql exclusive type)
• some fixes i found along the way, will likely clean those up and put them up as a separate PR

[Wulf]

some thoughts:

Let's explicitly group tests into 'generation tests' and 'compilation tests'.

I'm thinking test/generation/* and test/compile/*. We can add a README in the test folder to document the difference.

Also, where are we running the compilation currently? In other words, where is the simple_table_pg being compiled? I don't see any calls to cargo build in the the test.sh file.

The rest looks good to me though, thanks for the the simple_table split!

[hasezoey]

Also, where are we running the compilation currently? In other words, where is the simple_table_pg being compiled? I don't see any calls to cargo build in the the test.sh file.

because it is a workspace and they are members of it, it will get build with a cargo build in the root of the project

Let's explicitly group tests into 'generation tests' and 'compilation tests'.
I'm thinking test/generation/* and test/compile/*. We can add a README in the test folder to document the difference.

i am somewhat against this idea, because why generate 2 times if once is enough? the compile test would also need generation, would it not? (i dont like duplicating tests)

Edit: i just wanna note that i have nothing against adding and testing examples (like #111), but those are likely not enough to actually test all features dsync has

---

also i am not quite happy with the current implementation of this PR, reasons:

• it will get build with a cargo build in the root of the project (all tests get build), unless explicitly specifying what to build
• cargo fmt ignores the @generated and will still try to format all the tests
• all tests that are "build-able" need to have --manifest-path argument set in the test.sh
• using a different entry-point than the root of the project (workspace), will make it (with the current language server) annoying to work on the tests; because the (current language server) only checks and reports errors / warnings for things reachable from the root Cargo.toml's entry points
• versions are currently not pinned (like diesel) which could make it fail abruptly without warning (and annoying to figure out what changed)

[Wulf]

ah, understood!

I also changed my mind about splitting the tests: all our tests should be generate+compile tests (when possible)

What I don't like about #111 is that the tests are written/compiled as examples which could confine us (perhaps in a good way?)

[hasezoey]

What I don't like about #111 is that the tests are written/compiled as examples which could confine us (perhaps in a good way?)

i dont understand what this would confine us about, i think it is a good way to demonstrate both library usage (though the PR itself would still need to update some things) and for us to get notified directly if the examples get broken (like not compiling anymore), though generation output is not affected by this

---

I also changed my mind about splitting the tests: all our tests should be generate+compile tests (when possible)

if we should go ahead with the current PR, i would like to know what you think about the "also i am not quite happy with the current implementation of this PR, reasons", should the tests be in a separate workspace instead of referencing the main Cargo.toml? (but this would make tooling like vscode-rust-analyzer require opening that folder as the entry point to actually allow proper rust editing)

[Wulf]

hey @hasezoey, hope all has been well and that you've enjoyed the holiday season (happy new year!). I revisited your reasoning and I agree -- it's better for us not to include the tests as part of the workspace.

In this case, let's move forward with #111 for now, we can change our approach later on as necessary.

[hasezoey]

Updated to latest master and separated out the workspace to be a test-specific workspace, also updated the test scripts to easily generate & compile things

[hasezoey]

converted test from #126 to be compile-able, found out that this test still used unsigned, which postgres (at least in diesel) does not support

[hasezoey]

marked as "ready for review", because though there are still some points i dont quite like, it is better than before and definitely provides us with better robustness than without it

this pr does not mean that #111 shouldnt be merged too

[Wulf]

I've noticed this in the other tests -- do you recall why this was necessary? Just curious :)

[hasezoey]

see #94, #109 and then #116, which lead to the other PRs which changed to use diesel:: basically everywhere (but not completely yet because of trait imports, see #94 (comment))

TL;DR: we are mostly there to completely remove this, but having this PR (#114) will make it a lot easier figuring out which traits need to be imported / the absolute path necessary

[Wulf]

🙌