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

Default to panicking if a test accesses a real db #1460

Closed
matt2e opened this issue May 10, 2024 · 3 comments · Fixed by #1462
Closed

Default to panicking if a test accesses a real db #1460

matt2e opened this issue May 10, 2024 · 3 comments · Fixed by #1462
Assignees

Comments

@matt2e
Copy link
Collaborator

matt2e commented May 10, 2024

We now have ftltest.WithDatabase(...) which switches the DSN to a <name>_test db (and wipes the db tables).
It can be hard to tell when writing tests if a real db is accidentally being used.
example: I'm trying to move pfi tests to use test dbs but it's pretty opaque without looking through each package carefully.

If there is a legitimate usecase for touching db's that havent been swapped with ftltest.WithDatabase we could add a option to allow it?

Thoughts?

@github-actions github-actions bot added the triage Issue needs triaging label May 10, 2024
@alecthomas
Copy link
Collaborator

How would we do this?

@matt2e
Copy link
Collaborator Author

matt2e commented May 10, 2024

  • ftltest.WithDatabase(...) could mark the ModuleContext.Database as a test db.
  • module context already knows if it is in a test context from ftltest
  • So in modulecontext.GetDatabase(name), we can check for the case of moduleCtx.isTest && !database.isTestDB

@alecthomas
Copy link
Collaborator

Sounds like a good idea to me.

@matt2e matt2e self-assigned this May 10, 2024
@github-actions github-actions bot removed the triage Issue needs triaging label May 10, 2024
matt2e added a commit that referenced this issue May 10, 2024
fixes #1460
Issue says to panic, but in the end I just returned an error

Example error message:
> accessing non-test database "oidcauth" while testing: try adding
ftltest.WithDatabase(db) as an option with ftltest.Context(...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants