mocha icon indicating copy to clipboard operation
mocha copied to clipboard

🐛 Bug: HTML report pass/fail filters do not work correctly while testsuite runs

Open TimvdLippe opened this issue 6 years ago • 6 comments

The HTML report can filter by failures. Clicking on the Mocha stats in the top-right to inspect the failures, while the test suite is still running, all suites that have all passing tests are still added to the page. E.g. if you have a test that fails, clicking on failures shows that test. However, every consecutive suite that passes just fine is appended (just the name) to the report. Clicking on passes and then back on failures clears these names and again only shows the failures.

Therefore, I would like to have the failures page filter out fully passing test suites while the full test suite is running. I think that after this line the code from here must be run to then hide the suite name?

TimvdLippe avatar Aug 07 '17 18:08 TimvdLippe

Thanks for spotting this; I don't know I'd have ever thought to click the filters while the report is still running! I assume a similar problem occurs for failing suites when the passing filter is turned on and should only show passing tests/suites?

My understanding is that the tests are hidden by CSS based on a class applied to the root of the report, while the suites are hidden by JS actions upon them only when the filter button is clicked. I presume that's because whether a suite should be hidden depends on whether there are passing/failing tests in its descendants and CSS can't do anything based on descendants. But the fact that this is done by mutating individual suites each time the filter is set means that new suites aren't affected.

If my understanding is correct, then I'd like to fix this by:

  • When a passed/failed test is added, walk the DOM structure of suites to add a "contains passed/failed test" class to each ancestor suite of that test.
  • Update the CSS to hide suites that descend from the pass/fail class on the root and do :not() have the "contains passed/failed test" class.
  • Remove the suite stuff from the JS that fires on clicking the filter button.

This should achieve the same end result as directly hiding/showing suites on the addition of passed/failed tests, but since it separates the state of the filter from whether a suite should be shown or hidden in any given filter state, the filter state is simplified to one CSS class and the JS on the whole doesn't get more complicated (we'd replace the suite hiding/showing code with suite pass/fail-containing-marking code, rather than keeping the filter-state-finding and suite hiding/showing code but also adapting it to add a variation in another place), and the whole thing involves less direct manipulation by individual mutation of the end result and more semantic underlying behavior from which the right result should emerge in any given situation. That's what I think I'm going to advocate for at the moment anyway. ;^)

Either way, I'll see if I can devise tests for tests and suites being hidden correctly when the filter is activated before the relevant tests are added, and add them to #2900 (actually, ought to be tests in there that I can adapt for this just by moving the click-filter line). Any that aren't passing I can mark as pending and we can try either or both possible solutions and validate them by un-skipping those tests!

ScottFreeCode avatar Aug 07 '17 22:08 ScottFreeCode

Sounds good, I have no personal experience with the code :stuck_out_tongue: Looking forward to the fix!

TimvdLippe avatar Aug 09 '17 21:08 TimvdLippe

@ScottFreeCode could you give an update for this issue? It seems that #2900 is blocked on #2886. In turn #2886 was blocked on #2890, but that has been resolved as of yet. Does that mean #2886 can be moved forward to unblock #2900 as well? Looking forward to your reply 😄

TimvdLippe avatar Oct 22 '17 21:10 TimvdLippe

A lot of the stuff that #2886 and #2900 touched was overhauled in release 4, so I need to figure out how to rewrite each of those PRs to keep their intended changes while applying them to the current version of Mocha -- and then we'll find out for sure whether #2890 really did get fixed or only some other types of hanging. It shouldn't take too long once I can really devote some time to it but it's the kind of comprehensive analysis that I can't easily do in bits and pieces, so it's going to have to wait till I have a free evening or two to devote to it.

ScottFreeCode avatar Oct 23 '17 16:10 ScottFreeCode

While making a new html-electron reporter I tend to try to streamline the original html reporter.

I added a link to the duration display to enable showing only slow tests (slow/medium) and also an 'all' link

To get the filtering working while tests are still running I created a function updateHiding() This is then called from each of the filter events and from inside the updateStats() function.

/**
 * Stats template.
 */

var statsTemplate = '<ul id="mocha-stats">' +
  '<li class="progress"><canvas width="40" height="40"></canvas></li>' +
  '<li class="all"><a href="javascript:void(0);">all:</a></li>' +         // <- added link
  '<li class="passes"><a href="javascript:void(0);">passes:</a> <em>0</em></li>' +
  '<li class="failures"><a href="javascript:void(0);">failures:</a> <em>0</em></li>' +
  '<li class="duration"><a href="javascript:void(0);">duration:</a> <em>0</em>s</li>' + // <- added link
  '</ul>';

var playIcon = '&#x2023;';

/**
 * Initialize a new `HTML` reporter.
 *
 * @api public
 * @param {Runner} runner
 */
function HTML_Electron (runner) {
  Base.call(this, runner);

  var self = this;
  var stats = this.stats;
  var stat = fragment(statsTemplate);
  var items = stat.getElementsByTagName('li');
  var allLink = items[1].getElementsByTagName('a')[0];  // <- added link and indexes adjusted
  var passes = items[2].getElementsByTagName('em')[0];
  var passesLink = items[2].getElementsByTagName('a')[0];
  var failures = items[3].getElementsByTagName('em')[0];
  var failuresLink = items[3].getElementsByTagName('a')[0];
  var duration = items[4].getElementsByTagName('em')[0];
  var durationLink = items[4].getElementsByTagName('a')[0];  // <- added link
  var canvas = stat.getElementsByTagName('canvas')[0];
  var report = fragment('<ul id="mocha-report"></ul>');
  var stack = [report];
  var progress;
  var ctx;
  var root = document.getElementById('mocha');

  if (canvas.getContext) {
    var ratio = window.devicePixelRatio || 1;
    canvas.style.width = canvas.width;
    canvas.style.height = canvas.height;
    canvas.width *= ratio;
    canvas.height *= ratio;
    ctx = canvas.getContext('2d');
    ctx.scale(ratio, ratio);
    progress = new Progress();
  }

  if (!root) {
    return error('#mocha div missing, add it to your document');
  }

  // ******************* 
  const reState = / fail| pass| time/g

  function updateHiding() {
    unhide()
    var found = report.className.match(/pass|fail|time/)
    if (found) switch (found[0]) {
      case 'pass':
        hideSuitesWithout('test pass')
      break;
    
      case 'fail':
        hideSuitesWithout('test fail')
      break;
    
      case 'time':
        hideSuitesWithNoneOf(['test medium','test slow'])
      break;
    }
  }
  // show all
  on(allLink, 'click', function (evt) {
    evt.preventDefault();
    report.className = report.className.replace(reState, '');
    updateHiding()
  });

  // pass toggle
  on(passesLink, 'click', function (evt) {
    evt.preventDefault();
    var name = (/pass/).test(report.className) ? '' : ' pass';
    report.className = report.className.replace(reState, '') + name;
    updateHiding()
  });

  // failure toggle
  on(failuresLink, 'click', function (evt) {
    evt.preventDefault();
    var name = (/fail/).test(report.className) ? '' : ' fail';
    report.className = report.className.replace(reState, '') + name;
    updateHiding()
  });
  
  // duration toggle
  on(durationLink, 'click', function (evt) {
    evt.preventDefault();
    var name = (/time/).test(report.className) ? '' : ' time';
    report.className = report.className.replace(reState, '') + name;
    updateHiding()
  });
  

. . .


  function updateStats () {
    // TODO: add to stats
    var percent = stats.tests / runner.total * 100 | 0;
    if (progress) {
      progress.update(percent).draw(ctx);
    }

    // update stats
    var ms = new Date() - stats.start;
    text(passes, stats.passes);
    text(failures, stats.failures);
    text(duration, (ms / 1000).toFixed(2));

    updateHiding()  // <-  !!!
  }


/**
 * Check for suites that do not have elements
 * with any of `classnames`, and hide them.
 *
 * @param {text[]} classnames
 */
function hideSuitesWithNoneOf (classnames) {
  var suites = document.getElementsByClassName('suite');
  for (var i = 0; i < suites.length; i++) {
    var count = 0;
    for (var j = 0; j < classnames.length; j++) {
      var els = suites[i].getElementsByClassName(classnames[j]);
      count += els.length
    }
    if (count === 0) {
      suites[i].className = suites[i].className.replace(
        'suite',
        'suite hidden'
      );
    }
  }
}

I hope this will be of help for furthering the project. @ScottFreeCode

HeilTec avatar Nov 17 '17 21:11 HeilTec

@ScottFreeCode Would you be so kind giving another update on this issue? Thanks!

TimvdLippe avatar Sep 22 '18 20:09 TimvdLippe