qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
60 stars 18 forks source link

add status to suites #20

Closed fcarstens closed 9 years ago

fcarstens commented 9 years ago

This PR fixes #19. The suite status is available at globalSuite.getTotal().status. test.errors should already be there?

jzaefferer commented 9 years ago

I'm still not happy with the getTotal() method. Can we just run this inside the constructor and add the properties to the instance? Though according to our current draft, there should be runtime, suites, tests and status.

fcarstens commented 9 years ago

This is problematic if we want to pass Test and Suite objects on the start events as well. We would then need to recalculate them somehow later on... Alternatively, we could use es5 getters:

class Suite {
  constructor(){
    this.tests = [1,2,3]; // Simple Demo
  }
  getAllTests(){
    return this.tests;
  }
  get total(){
    return this.getAllTests().length;
  }
}
Object.defineProperties(Suite.prototype, {
  toJSON: {
    value: function(){
      let ret = {};
      for(let x in this){
        ret[x] = this[x];
      }
      return ret;
    }
  },
  total: {
    enumerable: true
  }
})

var s = new Suite();
console.log(s.total, JSON.stringify(s));
// 3 {"tests":[1,2,3],"total":3}

What do you think?

jzaefferer commented 9 years ago

Okay, let's give that a try.

fcarstens commented 9 years ago

I implemented this on top of the test stuff: https://github.com/fcarstens/js-reporters/tree/suite_summary Shall I open a new PR?

jzaefferer commented 9 years ago

Maybe rebase #23 into a set of reasonably atomic commits, than add this on top?

fcarstens commented 9 years ago

Included in #23.