Following contribution guidelines, test database not migrated

[autodidaddict]

I was following step-by-step in the CONTRIBUTING file and doing things in the order specified. It did not migrate the test database, so when I went to run the tests via cargo test, it failed complaining about a missing relationship.

My workaround was to set the DATABASE_URL to the test database URL and run the diesel migrate , then set the url back to the regular one. I was then able to run cargo test. I don't know whether there's a problem in the tests or whether the problem is in the instructions for contributing. Either way, this could be discouraging if someone stumbles on this and doesn't think to hack the URLs for a workaround.

[jtgeibel]

I think my recent PR #987 will resolve this. It ensures that the build script is run again when the timestamp on the migrations/ directory changes.

[carols10cents]

@jtgeibel unfortunately i don't think your PR will help this :( Here's how I'm able to reproduce, I'm not sure exactly what steps @autodidaddict took but I suspect it's similar to this:

• Run cargo test once (which compiles the tests)
• psql and drop database cargo_registry_test; create database cargo_registry_test;
• do not change any files, do not add any migrations, do not pass go, do not collect $200
• Run cargo test again (which does not compile the tests since they were already compiled)
• The tests will fail because the tables don't exist, since the migrations weren't run :(

I think we want to attempt to run the migrations every time we run cargo test, but I don't know how to do that or why we aren't already doing that.

[jtgeibel]

I originally ran into this in #762 and the build script was added in #767. I think the main issue is that there is no way to wire in to cargo test to run code before running all the test binaries and I also don't think it is possible to force cargo to run the test binaries in a particular order. On the other hand, the build script has errored on the side of checking for pending migrations more often than is necessary, such as during normal builds.

My guess to the problem here is that the steps in the Starting the server and the frontend section appear before the Running the backend tests section. So when following the steps in order, the first time the build script is run the test database does not yet exist. Once the test database is created, there is no timestamp change to cause the migrations to run.

I wonder if we could resolve this by simply swapping the order of these two subsections (although currently the first one is h5 and the second h4).

Also related is that the TEST_DATABASE_URL environment variable is typically defined in .env. So I don't think cargo will ever see a change (or even the existence of) this variable making the cargo:rerun-if-env-changed=TEST_DATABASE_URL line a no-op. Of course we could add .env to the list of files that cargo watches for changes.

The only alternative I see is going back to an implementation similar to what existed before #767 and including the hook in each test binary so that it doesn't matter which order cargo executes them in.

[Turbo87]

@LawnGnome since you went through the contribution guidelines a while ago, do you know if this issue is still relevant? feel free to close, if it is not :)

[LawnGnome]

Pretty sure this is all good now.