Decompress files for config_install testing

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Do we need to get the default_config_hash keys in the config?

I think that's a valid question, but since this is a security issue should that be a followup since this is just exporting the tars as is?

The only config change I did was add a missing newline so the validation would work.

I'm not sure how to scope security issues.

That I cannot answer unfortunately. But +1 for breaking out though

Will go ahead and mark to keep this training moving because I do believe breaking out makes a lot of sense.

The default hash is my only concern

Yes we do need to keep the default config hash - these are exports from real sites and so would be there.

Added a comment to the MR... we need to think about contrib and custom that are using the abstract class. The MR comment contains a plan on how to cope with that.

Is there any chance I can just deprecate the method getConfigTarball and add getConfigLocation.

Creating the new class and swapping things over should be straightforward, but almost every test using it is breaking now even though it's an exact copy of the previous version of the MR. The only change is I had to add a default theme.

@nicxvan the PHPStan warnings a bogus - we need to file an MR to fix \mglaman\PHPStanDrupal\Rules\Drupal\Tests\BrowserTestBaseDefaultThemeRule against that project. I'm really not a huge fan of that rule - testing tests is OTT in my opinion - especially when if you run the test it will tell you it is missing a default theme. Anyway... the quick fix here is to change the baseline and move on.

The reason tests are failing is because you extended the wrong class :) - fixed in my push.

Ah, I knew I was missing something obvious on the tests.

I already filed an issue against drupal phpstan for mglaman.

Tests are now passing thanks!

Seems feedback has been addressed.

CR reads well, always appreciate the before/after code examples.

Given this is a test only change backported to 10.4.x - makes it easier to maintain 10 and 11 compatible modules and update your code.

Committed 0426157 and pushed to 11.x. Thanks!
Committed cb746fd and pushed to 11.0.x. Thanks!
Committed fb01cca and pushed to 10.4.x. Thanks!

OMG, I cannot believe we've dropped this. Thank you

P

Published the CR.

xjm → closed merge request !8461

Sorry for noise; somehow either commenting about the CR or closing the MR ended up crediting me for this issue which is not correct. 😂 Fixed now.

Automatically closed - issue fixed for 2 weeks with no activity.