Problem/Motivation

Today, it's only possible to perform a coverage analysis on contributed modules. We have no idea about the coverage of the tests on the core modules.

Proposed resolution

A user should be able to perform a coverage analysis on core modules.

Remaining tasks

  • Identify how often we should perform these tests
  • Identify if we should run them during a nightly build
  • Make sure that DrupalTI can deal with this
  • Perform code changes

Comments

legovaer created an issue. See original summary.

legovaer’s picture

Assigned: Unassigned » legovaer

  • legovaer committed 8da17eb on 2804855-allow-core-modules
    Issue #2804855: Perform coverage analysis on core modules
    
legovaer’s picture

Pushed changes so far as creating automated tests has an increased priority at this point in time.

legovaer’s picture

Assigned: legovaer » Unassigned
jonathan1055’s picture

StatusFileSize
new72.45 KB

Just had a browse at http://cgit.drupalcode.org/drupal_coverage_core/commit/?id=8da17eb
I am not familiar with the code at all, and have not run anything obviously, but I noticed one liitle change that you might have meant to do differently:
analyzeForm
You have added $machine_name as the key in the foreach, but not actually used it. Did you mean to have $modules[$machine_name] = $module['name'] instead?

As I said, this is just from looking at some of the code. I have no idea exactly what all this is doing, but I just spotted that you must have made that change for a purpose.

legovaer’s picture

Jonathan,

Thanks for looking into this. The last commit was my work in progress on this issue. As more people are starting to work on this project, I had to switch to creating the unit tests for this project.

Your point is actually very valid and should get fixed.

In the meantime, I gave you access to the project on Acquia and I've updated the project page so that other people can ask for an invite for Acquia as well.

legovaer’s picture

Version: » 8.x-1.x-dev
Status: Active » Needs work
legovaer’s picture

Status: Needs work » Needs review
StatusFileSize
new12.18 KB

Fixed #6 and made sure that we can test D7 core modules.

Status: Needs review » Needs work

The last submitted patch, 9: perform_coverage-2804855-9.patch, failed testing.

legovaer’s picture

Status: Needs work » Needs review
StatusFileSize
new13.08 KB
new1.13 KB

Status: Needs review » Needs work

The last submitted patch, 11: perform_coverage-2804855-11.patch, failed testing.

legovaer’s picture

Status: Needs work » Needs review
StatusFileSize
new11.42 KB
new0 bytes

Added the config factory to the argument of the ModuleManager service.

Status: Needs review » Needs work

The last submitted patch, 13: perform_coverage-2804855-13.patch, failed testing.

legovaer’s picture

Status: Needs work » Needs review
StatusFileSize
new15.08 KB
new11.42 KB
bramdriesen’s picture

Status: Needs review » Reviewed & tested by the community

Looks good! :) great job

jonathan1055’s picture

Status: Reviewed & tested by the community » Needs work

Referring to my point back in #6 you say you fixed it in #9 but the original change still appears in the latest patch

-    foreach ($module_data as $module) {
-      $modules[] = $module['name'];
+    foreach ($module_list as $module) {
+      $modules[$module['name']] = $module['name'];
     }

Maybe you changed your mind and decided to leave it as-is, but just thought I would mention it, in case this got missed in your re-roll for 8.x-1.x

legovaer’s picture

Status: Needs work » Needs review
StatusFileSize
new15.1 KB
new454 bytes

Right, I missed that! Thanks for pointing out!

bramdriesen’s picture

Status: Needs review » Reviewed & tested by the community

#17 looks good, for the rest no changes since last time

  • legovaer committed a85c503 on 8.x-1.x
    Issue #2804855 by legovaer, jonathan1055: Perform coverage analysis on...
legovaer’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone!

Status: Fixed » Closed (fixed)

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