Whilst working on an issue I notice SchemaCheckTrait::checkConfigSchema() can fail to report errors.

CommentFileSizeAuthor
#7 2287193.7.patch3.5 KBalexpott
#7 2287193-test-only.7.patch3.02 KBalexpott
#1 2287193.1.patch769 bytesalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
2287193.1.patch769 bytes

Let's see what fails.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Normal » Major

Oh my.
Do we have tests for this?
This one is no longer just a test helper, since it was moved out...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review

Guessing this will pass. I really hope so.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: +Needs tests

That's pretty classic. Test coverage possible?

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: checkConfigSchema can fail to report errors » checkConfigSchema() can fail to report errors
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

Looks good :) We could theoretically add a config file with some invalid schema in elements other than the first and see how that acts before/after patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2287193-test-only.7.patch3.02 KB
2287193.7.patch3.5 KB

With a test. The test only patch is also the interdiff :)

The last submitted patch, 7: 2287193-test-only.7.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Yay, looks good, thanks!

  • Commit 7872b9a on 8.x by catch: 
    Issue #2287193 by alexpott: Fixed checkConfigSchema() can fail to report...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

This looks fine and is blocking another critical issue.

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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