There are a couple of warnings in the phpunit coverage tests. The errors in UrlTest and TokenUnitTest where typo's, The errors in the filesystem tests are because having 2 defaults doesn't make sense. Splitting the tests provides more clear coverage as well.

CommentFileSizeAuthor
#9 test-coverage-errors-9.patch6.38 KBneclimdul
#3 test-coverage-errors.patch6.86 KBneclimdul
test-coverage-errors.patch7.29 KBneclimdul

Comments

neclimdul’s picture

neclimdul
He/Him
commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, test-coverage-errors.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review
StatusFileSize
new test-coverage-errors.patch6.86 KB

Great minds apparently. #2245185: Fix invalid @covers usage did one of the typo fixes and a different approach to the File tests so this is basically a follow up. I would still like to split the File tests to make it clear what's being tested.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community 
diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php
copy from core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php
copy to core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php

Note to future reviewers, this is a copy not a move.

I think it looks good, nice finds.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: test-coverage-errors.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review

3: test-coverage-errors.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

back to rtbc.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: test-coverage-errors.patch, failed testing.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review
StatusFileSize
new test-coverage-errors-9.patch6.38 KB

Straight re-roll. Don't know what the failure was, the merge had no conflicts.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs review » Reviewed & tested by the community

going to be bold and bump this back to rtbc.

  • Commit 363445a on 8.x by webchick: 
    Issue #2247595 by neclimdul: Fix phpunit coverage warnings.
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

This diff looked like it was deleting a crap-ton of tests, but:

copy from core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php
copy to core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php

clears that up.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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