Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#12574 closed (fixed)

all tests pass when there broken tests becase they are skipped

Reported by: CarlFK Owned by: nobody
Component: Testing framework Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If a test is broken (like typo in models.py)
and you run the full set of tests:

django-trunk/tests$ ./runtests.py --settings=sets_sqlite3

the broken test gets skipped
and so you get:

----------------------------------------------------------------------
Ran 1390 tests in 243.340s
OK
----------------------------------------------------------------------

Which makes you think "oh boy! all my tests passed!" and go dancing in the street.

But no! the very test you were most concerned about never got run.
If you dig around in the output, you will see the error.
If you see OK, you really have no reason to do that.

Here is why:
(trimmed down version)

        try:
            mod = load_app(model_label)
            if mod:
                settings.INSTALLED_APPS.append(model_label)
        except Exception, e:
            sys.stderr.write("Error while importing %s:" % model_name + ''.join(traceback.format_exception(*sys.exc_info())[1:]))

Looking at the change history, it seems this is the result of some refactoring that resulted in this hiding place for bad tests. The "continue" command is unnecessary, given it is at the end of the codeblock.

Seems the fix is to remove the try/except:err.write and let the error get thrown.

btw, the comment for

if not test_labels or model_name in set([label.split('.')[0] for label in test_labels]):

is screwy. so is the code. Neither read well. if you fully understand it, it would be great if it could be cleaned up so I don't get a headache.

Attachments (2)

runtests.diff (1.7 KB) - added by CarlFK 6 years ago.
12574.diff (1.7 KB) - added by ramiro 6 years ago.
Same patch plus moving calculation of the set of test outside of a loop

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by CarlFK

comment:1 Changed 6 years ago by CarlFK

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

to see the bug in action:

echo foo >> modeltests/empty/models.py
./runtests.py --settings sets_sqlite -v 1

If you do -v 0 (the default), then you will see the stack dump ending in

NameError: name 'foo' is not defined

but also see: OK (which means 'all tests passed') (which is the bug).

Changed 6 years ago by ramiro

Same patch plus moving calculation of the set of test outside of a loop

comment:2 Changed 5 years ago by ericholscher

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(In [13616]) Fixed #12574 -- Removed an unnecessary exception catch from the system runtest script, which could hide failing tests. Thanks to CarlFK for the report, and Ramiro Morales for the polish.

comment:4 Changed 5 years ago by russellm

(In [13617]) [1.2.X] Fixed #12574 -- Removed an unnecessary exception catch from the system runtest script, which could hide failing tests. Thanks to CarlFK for the report, and Ramiro Morales for the polish.

Backport of r13616 from trunk.

Note: See TracTickets for help on using tickets.
Back to Top