search

spacetelescope / pandokia

Regression tests framework.
Other
2 stars 4 forks source link

Consolidate tra_exception data #26

Closed jhunkeler closed 6 years ago

jhunkeler commented 6 years ago

le_whoops.pdf

Tests used:

import nose

def test_long_exception_traceback():
    output = ''
    max_length = 80
    count = 0
    for x in range(0x1000):
        if count < max_length:
            output += '*'
        else:
            count = 0
            output += '\n'
        count += 1

    raise ValueError(output)

def test_exception_traceback():
    raise ValueError('I recieved the wrong value. Le whoops.')

Resolves #25

jhunkeler commented 6 years ago

Hey @oiintam and @cdsontag Does this PDF look more like what you're expecting to see?

cdsontag commented 6 years ago

Thanks @jhunkeler! Question - looks from that PDF like the tra_exception got fixed (yay!) and is as expected, but a now new TRA called tra_Exception (cap-E) got added somehow? We don't need that for ETC stuff, do you need that for other DATB tests?

It would look more correct to me without tra_Exception, but maybe I am misunderstanding something.

oiintam commented 6 years ago

I agree with Chris. On an unrelated side note, I like your naming of the pdf. XD

jhunkeler commented 6 years ago

@oiintam :grin: hehe ... Sometimes there's no other way to describe it!

@cdsontag I never noticed this however tra_Exception and tra_exception have been there for over seven to eight years. Check this out...

$ grep -rE '\[\S[eE]xception\S]'
pandokia/helpers/pytest_plugin.py:                    item.pandokia.tra['Exception'] = call.excinfo.exconly()
pandokia/helpers/pytest_plugin.py:                item.pandokia.tra['Exception'] = call.excinfo.exconly()
pandokia/helpers/pycode.py:                self.tra['exception'] = str(exvalue)
pandokia/helpers/runner_minipyt.py:        tra['exception'] = exception_str
pandokia/helpers/runner_minipyt.py:        tra['exception'] = exception_str
pandokia/helpers/runner_minipyt.py:        tra['exception'] = exception_str
pandokia/helpers/runner_minipyt.py:        tra['exception'] = exception_str
pandokia/helpers/nose_plugin.py:            tra['Exception'] = exc
$ find pandokia -type f -name "*.py" -print | xargs -n1 -I'{}' git --no-pager blame -f -n -L '/tra\[\S[eE]xception\S\]/' "{}" 2>/dev/null |grep -E 'tra\[\S[eE]xception\S\]'
9e79eab6 pandokia/helpers/pytest_plugin.py 351 (sienkiew        2011-12-02 23:02:19 +0000 458)                     item.pandokia.tra['Exception'] = call.excinfo.exconly()
45e866c8 pandokia/helpers/pytest_plugin.py 442 (sienkiew        2012-04-06 21:46:03 +0000 480)                 item.pandokia.tra['Exception'] = call.excinfo.exconly()
6d712b4a pandokia/helpers/pycode.py 413 (sienkiew        2012-04-06 20:08:01 +0000 541)                 self.tra['exception'] = str(exvalue)
92ea9eff pandokia/helpers/runner_minipyt.py 285 (sienkiew        2010-10-01 19:16:46 +0000 338)         tra['exception'] = exception_str
92ea9eff pandokia/helpers/runner_minipyt.py 361 (sienkiew        2010-10-01 19:16:46 +0000 428)         tra['exception'] = exception_str
92ea9eff pandokia/helpers/runner_minipyt.py 436 (sienkiew        2010-10-01 19:16:46 +0000 517)         tra['exception'] = exception_str
92ea9eff pandokia/helpers/runner_minipyt.py 670 (sienkiew        2010-10-01 19:16:46 +0000 777)         tra['exception'] = exception_str
90b460fe pandokia/helpers/nose_plugin.py 283 (sienkiew        2010-05-11 15:33:47 +0000 360)             tra['Exception'] = exc

As far as I can tell the tra_[eE]xception thing is a builtin/hard-coded feature for each plugin. I've never understood the rationale behind it though, considering the full log output includes this information.

Maybe the original intent was to do away with dumping the log and parse everything out into specialized fields... I'm not sure. Should I normalize everything to Ex or ex or leave it alone?

cdsontag commented 6 years ago

I never noticed this however tra_Exception and tra_exception have been there for over seven to eight years. Check this out...

@jhunkeler - wow, that is wild. I am inclined to think that is totally an 8-year-old typo (and thus it should all be changed to just lower-case e tra_exception), but I'd like to see what @vglaidler has to say. She wrote some of this with Mark S. and she may know of a reason/intention they had to have both tra_exception and tra_Exception.

If they do get consolidated though, in the end, of course we still want tra_exception to not include stdout captured but only the exception itself.

Thanks Joe!

vglaidler commented 6 years ago

Interesting, looks like exception was used for home-grown test runners, & Exception was used for plugins, but I don't recall any conscious decision about that. Let me take a closer look when I get into the office - I'm leaving now.

vglaidler commented 6 years ago

Thanks for your quick attention to this, @jhunkeler , and for including the test code. I've taken a close look & observe the following:

To answer your question above

As far as I can tell the tra_[eE]xception thing is a builtin/hard-coded feature for each plugin. I've never understood the rationale behind it though, considering the full log output includes this information.

The rationale is that, if you have 600 erroring tests, it's really helpful to be able to quickly check the actual exception for all of them at once, instead of having to read all 600 test logs. This is the kind of analysis that was Pandokia's originating use case.

One last thing - we have an ETC test harness that also interacts directly with exception information, so it's possible that we may discover another issue when we try using Pandokia to run our tests.

vglaidler commented 6 years ago

Hi @jhunkeler , can you please run this test and post the results so we can determine whether the initial problem is solved? Thanks.

In order to test the behavior that Chris is concerned about, can you please cause both tests to print something to stdout before they error?

jhunkeler commented 6 years ago

PDF report: le_whoops2.pdf

Tests used:

import nose
import sys

def textangle(length=0x200, ch='*'):
    output = ''
    max_length = 80

    if length < max_length:
        length = max_length

    count = 0
    for x in range(length):
        if count < max_length:
            output += ch
        else:
            count = 0
            output += '\n'
            continue

        count += len(ch)

    return output

def test_long_exception_traceback():
    output = textangle()
    raise ValueError(output)

def test_long_exception_stdout_traceback():
    output = textangle()
    output_tty = textangle(ch='_STDOUT_')
    print(output_tty)
    raise ValueError(output)

def test_exception_traceback():
    raise ValueError('I recieved the wrong value. Le whoops.')

def test_exception_failure_ignore_tra():
    assert 1 == 0, 'This should not be in the tra_exception field'

def test_exception_failure_ignore_tra_stdout():
    print('This should not be in the tra_exception field')
    assert 1 == 0, 'Nor should this be in the tra_exception field'

def test_passes():
    assert 0 == 0, 'Zero is not zero. Interesting.'
vglaidler commented 6 years ago

Thanks Joe! This looks good to me. I'm a bit concerned by the mysterious leading dot, but not enough to block the PR - we can file a ticket to track that down someday.

Please go ahead & merge, and then I believe we need an updated TP9 from Matt that includes the latest version - is that right @oiintam ?

oiintam commented 6 years ago

@vglaidler That is correct. Once this is merged, I will message Matt for an updated version of TP9. @jhunkeler Thank you very much for fixing this.

jhunkeler commented 6 years ago

@vglaidler + @oiintam you're welcome... sorry for the delay there. (Being implicitly subscribed to every repo makes it difficult to see messages directed to me.)