[WIP] Issue #130

[timyhou]

Updated serialize_report and unserialize_report to allow passing of additional exception info using ReprExceptionInfo. We could also use ExceptionChainRepr instead actually. I mostly just took a suggestion from mszpala and ran with it.

This addresses issue #130.

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs:

• Make sure to include reasonable tests for your change if necessary

• Add a new news fragment into the changelog folder, following these guidelines:

  • Name it $issue_id.$type for example 588.bug

  • If you don't have an issue_id change it to the PR id after creating it

  • Ensure type is one of removal, feature, bugfix, vendor, doc or trivial

  • Make sure to use full sentences with correct case and punctuation, for example:

    Fix issue with non-ascii contents in doctest text files.
    

[RonnyPfannschmidt]

the approach looks interesting, unfortunately it broke the build at least on old pytest

@nicoddemus this one is in particular an issue since we started to use tox-travis - by now im relatively certain that i want to get rid of tox-travis, it breaks results in some cases

[timyhou]

I am taking a look at the failures. I admit I didn't test it out on older versions of python & pytest so I'll give it another go.

[RonnyPfannschmidt]

@timyhou i stongly suggest using tox locally - we may be fine with dropping pytest 2.x support over this

@nicoddemus oppinions?

[nicoddemus]

we may be fine with dropping pytest 2.x support over this

I agree, we might require pytest >= 3 then

[timyhou]

Should I add a commit with an updated tox.ini to not bother testing pytest 2.x to let the Travis CI and appveyor checks try again?

[nicoddemus]

Yes please. 😁

[timyhou]

Sorry the commit history is going to get kind of messy as I realized I forgot to update the appveyor.yml and setup.py. If this becomes a thing, I promise to squash it properly.

[timyhou]

The test I am having trouble with right now is TestFunctional.test_xfail_passes. I can reproduce the failure locally through tox and by setting up an virtualenv.
I can see that the spawn.out file doesn't seem to have LOOPONFAILING in the log. It does mark the test as "xpass" however I am hesitant to have the test not verify the LOOPONFAILING part since I am unfamiliar with it.

The other test failures on Travis I haven't been able to reproduce locally though and the "fixed" themselves the last run on TravisCI? They seemed to only be coming from the pexpect configurations.

Those tests were:

• test_keyboard_interrupt_dist (only on py27). Similar issue where the spawn.out is missing text.
• test_looponfail_removed_test

Added: Resolved these issues. A lot of the failures had to do with bad tox.ini configurations and changes in tests due to pytest >= 3

[timyhou]

I made a branch with a a cleaner git history on my fork. Would it be better to close this PR and start a new one against the better branch?

[nicoddemus]

@timyhou you can push-force to this branch, the PR will update automatically.

[nicoddemus]

Or feel free to create to create a new PR, no problem with that. 👍

[timyhou]

Updated.

[nicoddemus]

Awesome, thanks! I checked locally and it works! 👍

[nicoddemus]

Hmm I might have pushed that merge button too soon, I could have sworn @RonnyPfannschmidt was ok with the change already. @RonnyPfannschmidt what do you think?

[RonnyPfannschmidt]

We should move this to pytest core eventually

[messa]

Hi, are you sure about this? :)

if 'reprcrash' and 'reprtraceback' in reportdict['longrepr']:

Maybe if I add some parenthesis how it is evaluated by Python:

if ('reprcrash') and ('reprtraceback' in reportdict['longrepr']):

[nicoddemus]

@messa oh good catch, this definitely should be:

if 'reprcrash' in reportdict['longrepr'] and 'reprtraceback' in reportdict['longrepr']:

Would you like to open a PR with this fix?

[messa]

@nicoddemus Done: #213