Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallel tests are broken #9

Open
benjamin-thomas opened this issue Nov 1, 2023 · 4 comments
Open

Parallel tests are broken #9

benjamin-thomas opened this issue Nov 1, 2023 · 4 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@benjamin-thomas
Copy link
Collaborator

I get random test failures, which I think are due to separate project's trying to setup the database at the same time.

For instance :

  • project A starts
  • project A : seeds the authors table
  • project B starts
  • project B: seeds the authors table --> duplicate key violation
  • project A cleanup
  • etc.

Quick-fix solution: build and/or test with one worker.


PGHOST=localhost PGDATABASE=caqti_study PGPORT=5433 dune build @all @runtest -j1

Or:

PGHOST=localhost PGDATABASE=caqti_study PGPORT=5433 dune build
PGHOST=localhost PGDATABASE=caqti_study PGPORT=5433 dune runtest -j1

It'd be nice to find a better solution. Maybe something like:

  • setup the database once (create and seed data for all the apps)
  • run each test into a transaction, then they wouldn't influence each other
@paurkedal
Copy link
Owner

Dune has a (locks VIRTUAL_PATH) attribute which can be used to avoid conflicting concurrent runs (cf the Caqti testsuite). But I'll think some more about it and comment, since I'm having similar issues with test suites depending on DBs (and not just in Caqti).

Another motivation I have for rethinking this is that if someone (like myself) already has set up access to a personal database, then we want to avoid accidentally creating tables in its public namespace when the PG* variables are unset.

@benjamin-thomas
Copy link
Collaborator Author

Thanks for the tip!

To avoid accidentally creating or deleting tables, I can require mandatory prefixed env vars, something like:

  • CAQTI_STUDY_PGHOST
  • CAQTI_STUDY_PGDATABASE
  • CAQTI_STUDY_PORT
  • etc.

Would that work for you?

Env vars are pretty flexible IMO, and make it so we never have to worry about committing sensitive data by accident.

We can simulate having something close to a config file for free using a tool such as envdir, from the daemontools package.

$ echo 1 >/tmp/env/CAQTI_HELLO
$ echo 2 >/tmp/env/CAQTI_WORLD
$ envdir /tmp/env/ env | grep CAQTI
CAQTI_WORLD=2
CAQTI_HELLO=1

# Or if that wasn't clear
$ envdir /tmp/env/ bash -c 'echo $CAQTI_HELLO'
1

So one could run the test suite like such:

envdir ~/env/caqti-study/ dune runtest

@paurkedal
Copy link
Owner

I think adding a prefix to the variables is good, since it allows people to set the variables globally if they wish, without affecting other applications. However, we would also need to drop the postgres:// fall-back, since that may point to the wrong database. But we could consider making the fall-back postgres://localhost:5433/local_db for convenience. It's already the default in the makefile, and I think it's safe enough, since the port is non-standard and the database name is unique enough (we could also call it caqtistudy or caqti_study).

(I didn't know about envdir, but an alternative is to put the environment variables in a script which executes the rest of the command-line at the bottom.)

@benjamin-thomas
Copy link
Collaborator Author

However, we would also need to drop the postgres:// fall-back, since that may point to the wrong database

Sounds good to me. I'll make this a failing condition.

an alternative is to put the environment variables in a script which executes the rest of the command-line at the bottom

Yes that's another way to do it. I'll write a little wrapper script to make things easy to manage.

@paurkedal paurkedal added bug Something isn't working enhancement New feature or request labels Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants