1125863 - Reports should differentiate enabled from added and disabled from removed

Status: RESOLVED INVALID

(Testing Graveyard :: Test Informant, defect)

Description

[Andrew Halberstadt [:ahal]]

It would be useful to see if tests are being disabled versus removed (i.e as part of a refactor). Likewise how often tests are being re-enabled versus added. It would probably also be good to have the ratio of enabled/disabled in the summary (which ignores all newly added or removed tests).

All this would give us a better view into how good we are about re-enabling disabled tests.

Comment 1

[Nathan Froyd [:froydnj]]

Test informant is all written in Python, correct?  So this project would be in Python?

Comment 2

[Andrew Halberstadt [:ahal]]

Yes, thanks. This particular bug would also need some modification to a jinja2 template (very similar to django templates).

Also just want to warn that bug 1124689 (which has another contributor working on it) could possibly result in a schema change which might have implications on this bug. Any changes as a results of that bug should be minor though, so I don't want that to discourage anyone from taking this.

Comment 3

[Alice Scarpa [:adusca]]

Attachment: Adding a disabled field to the database

I added a 'disabled' field to the database and I used that to separate removed tests from disabled ones. Am I on the right track?

Comment 4

[Alice Scarpa [:adusca]]

Comment on attachment 8557164 [details]
Adding a disabled field to the database

https://github.com/adusca/test-informant/commit/ab8957498c51bea41b13987a7d72d1e0f9834d73

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8557164 [details]
Adding a disabled field to the database

Good idea, but not quite. I was thinking that this would all be calculated on the report generating side:
https://github.com/mozilla/test-informant/blob/master/tools/formatters/base.py#L57

The goal is to differentiate "enabled" from "added" and "disabled" from "removed". To do this, we need to compare each test to the previous run. There are four scenarios we care about:

1. test was previously skipped, now it isn't => enabled
2. test was previously run, now it's skipped => disabled
3. test did not previously exist, now it's run => added
4. test was previously run, now it doesn't exist => removed

p.s feel free to refactor that code if you see a better way.

Comment 6

[Alice Scarpa [:adusca]]

Attachment: differentiating enabled from added and disabled from removed

Ops, I completely misunderstood what 'disabled' means. Second try.

Comment 7

[Andrew Halberstadt [:ahal]]

Comment on attachment 8557289 [details]
differentiating enabled from added and disabled from removed

https://github.com/adusca/test-informant/commit/ab8957498c51bea41b13987a7d72d1e0f9834d73#commitcomment-9563062

Comment 8

[Nathan Froyd [:froydnj]]

(In reply to Andrew Halberstadt [:ahal] from comment #5)
> There are four scenarios we care about:
> 
> 1. test was previously skipped, now it isn't => enabled
> 2. test was previously run, now it's skipped => disabled
> 3. test did not previously exist, now it's run => added
> 4. test was previously run, now it doesn't exist => removed

I think there's a fifth case here that should be added for completeness:

5. test did not previously exist, now it's skipped => disabled

I don't know how common this is, but I'm pretty sure I've seen a few tests just this year that fall into this bucket.  And since I'm thinking of it, there's a sixth case:

6. test was previously skipped, now it doesn't exist => removed