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

[NFC/Unit test] ReportTemplateTest - fix tests that assume setup from a separate test #20887

Merged
merged 1 commit into from
Jul 19, 2021

Conversation

demeritcowboy
Copy link
Contributor

Overview

Some tests in this file happen to work but shouldn't.

Before

Same

After

Same

Technical Details

When you turn off trigger-based logging, it (properly) doesn't delete all the log records, since the whole point of this logging is to have a forensic-like record of changes. Some of the tests in this file seem to assume a previous test has already turned on logging and that the tables will be there. While those previous tests turn off logging, they don't delete the log tables. So both the tables should be deleted and the later tests should do their own work and start from a clean slate.

If you look higher up around line 257 I've taken my cue from there.

Comments

I'm planning to add a test in here but it fails because of this.

@civibot
Copy link

civibot bot commented Jul 17, 2021

(Standard links)

@civibot civibot bot added the master label Jul 17, 2021
@seamuslee001
Copy link
Contributor

Looks good to me merging

@seamuslee001 seamuslee001 merged commit e9033ec into civicrm:master Jul 19, 2021
@demeritcowboy demeritcowboy deleted the lazytests branch July 19, 2021 23:38
@demeritcowboy
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants