Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-75880

Fix all @coversDefaultClass annotation orphaned uses

XMLWordPrintable

    • MOODLE_311_STABLE, MOODLE_400_STABLE, MOODLE_401_STABLE
    • MOODLE_311_STABLE, MOODLE_39_STABLE, MOODLE_400_STABLE
    • Hide

      A) Automated

      1. Verify that all the tests continue passing (Travis and GHA badges above, Also CI runs...).

      B) Manual (before the patch)

      1. Execute the following command (files having @coversDefaultClass and not having any @covers annotation, those are the ones being fixed here):

        grep -lr '@coversDefaultClass' | xargs grep -LP '@covers |@coversNothing ' | grep tests | wc -l
        

      2. Verify that you get the following number of files to be fixed:
        1. MOODLE_39_STABLE: 2
        2. MOODLE_311_STABLE: 9
        3. MOODLE_400_STABLE: 17
        4. master: 17

      C) Manual (after the patch)

      1. Execute the following command:

        grep -lr '@coversDefaultClass' | xargs grep -LP '@covers |@coversNothing ' | grep tests | wc -l
        

      2. Verify that the command returns "0" (zero). All them have been fixed.
      Show
      A) Automated Verify that all the tests continue passing (Travis and GHA badges above, Also CI runs...). B) Manual (before the patch) Execute the following command (files having @coversDefaultClass and not having any @covers annotation, those are the ones being fixed here): grep -lr '@coversDefaultClass' | xargs grep -LP '@covers |@coversNothing ' | grep tests | wc -l Verify that you get the following number of files to be fixed: MOODLE_39_STABLE: 2 MOODLE_311_STABLE: 9 MOODLE_400_STABLE: 17 master: 17 C) Manual (after the patch) Execute the following command: grep -lr '@coversDefaultClass' | xargs grep -LP '@covers |@coversNothing ' | grep tests | wc -l Verify that the command returns "0" (zero). All them have been fixed.

      While it may sound rare, @coversDefaultClass xxxx does not indicate any coverage at all.

      It's only an alias / shortcut to avoid having to write xxxx in all the individual @covers in a file (that can be really useful when the class is really long.

      Source: https://phpunit.readthedocs.io/en/9.5/annotations.html#coversdefaultclass

      So, using @coversDefaultClass xxxx is not enough to tell PHPUnit about coverage. The only annotation that does that is @covers (and @coversNothing, but it doesn't matter here.

      In core, we have a bunch of "alone / orphaned" @coversDefaultClass cases, not having any @covers annotation specified. This is the current list in master:

      $ ag '@coversDefaultClass' -l | xargs ag -vl '@covers |@coversNothing '
      course/format/tests/output/local/state/state_test.php
      admin/tool/admin_presets/tests/event/preset_reverted_test.php
      admin/tool/admin_presets/tests/event/preset_deleted_test.php
      admin/tool/admin_presets/tests/event/preset_previewed_test.php
      admin/tool/admin_presets/tests/event/preset_downloaded_test.php
      admin/tool/admin_presets/tests/event/preset_loaded_test.php
      admin/tool/admin_presets/tests/event/preset_exported_test.php
      admin/tool/admin_presets/tests/event/preset_imported_test.php
      admin/tool/admin_presets/tests/event/presets_listed_test.php
      lib/tests/content/export/exporters/course_exporter_test.php
      lib/tests/content/export/zipwriter_test.php
      lib/tests/content/export/exportable_items/exportable_stored_file_test.php
      lib/tests/content/export/exportable_items/exportable_textarea_test.php
      lib/tests/content/export/exportable_items/exportable_filearea_test.php
      lib/table/tests/local/filter/filter_test.php
      files/tests/local/archive_writer/zip_writer_test.php
      files/tests/archive_writer_test.php
      

      This issue is about to fix them by:

      1) When it's in a method (incorrect), change it to @covers.
      2) When it's in a class, consider if it really helps and add specific @covers ::method to every unit test, or just replace it by a simple @covers at class level.
      3) Ideally, add this to some checker, so it doesn't happen again. Note that moodle-cs is already checking for methods without any (class or method) coverage information, so it will cry for new cases. Maybe that's enough.

      Ciao

        1. after_patch_master_MDL-75880.png
          after_patch_master_MDL-75880.png
          14 kB
        2. after_patch_v311_MDL-75880.png
          after_patch_v311_MDL-75880.png
          14 kB
        3. after_patch_v39_MDL-75880.png
          after_patch_v39_MDL-75880.png
          14 kB
        4. after_patch_v400_MDL-75880.png
          after_patch_v400_MDL-75880.png
          14 kB
        5. stable_master_MDL-75880.png
          stable_master_MDL-75880.png
          13 kB
        6. stable_v311_MDL-75880.png
          stable_v311_MDL-75880.png
          13 kB
        7. stable_v39_MDL-75880.png
          stable_v39_MDL-75880.png
          14 kB
        8. stable_v400_MDL-75880.png
          stable_v400_MDL-75880.png
          13 kB

            stronk7 Eloy Lafuente (stronk7)
            stronk7 Eloy Lafuente (stronk7)
            Paul Holden Paul Holden
            Andrew Lyons Andrew Lyons
            John Edward Pedregosa John Edward Pedregosa
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 3 hours, 30 minutes
                3h 30m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.