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 fixes for the problem of quickform rules going missing #21725

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

demeritcowboy
Copy link
Contributor

@demeritcowboy demeritcowboy commented Oct 5, 2021

Overview

For a while now there's been a problem with certain form tests where the quickform rules go missing.

Before

Several attempts at workarounds.

After

Don't need workarounds

Technical Details

Note this line:

// Ugh. Need full bootstrap to enumerate classes.
$this->setUp();

That's in a dataprovider that runs BEFORE anything even starts. So there's some time-travel mindbending going on - calling setup before setup.

It's nice to have the dataprovider but moving it to a loop inside the actual function works and avoids the problems. It looks complicated in the github view but that's the main change here.

Comments

There is still one annotation remaining that starts a test in a separate process. That's a different issue and it doesn't seem to hurt at the moment, but may need revisiting.

@civibot
Copy link

civibot bot commented Oct 5, 2021

(Standard links)

@civibot civibot bot added the master label Oct 5, 2021
@demeritcowboy demeritcowboy changed the title [WIP] [NFC] Unit test fixes for the problem of quickform rules going missing Oct 5, 2021
@eileenmcnaughton
Copy link
Contributor

I do believe you have cracked it!

@eileenmcnaughton
Copy link
Contributor

And look at all those previous attempts to solve it!

@eileenmcnaughton eileenmcnaughton merged commit 30b363b into civicrm:master Oct 5, 2021
@demeritcowboy demeritcowboy deleted the maxfilesize2 branch October 5, 2021 21:18
@demeritcowboy
Copy link
Contributor Author

Yay!

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