gnecula / bond

Spy-based testing library.
http://necula01.github.io/bond/
Other
7 stars 3 forks source link

bond_reconcile can be invoked even on test failure #14

Closed gnecula closed 9 years ago

gnecula commented 9 years ago

Right now on test failure, we do not invoke reconcile, and we cannot see the differences.

We will add another parameter to bond_reconcile: --no-save="Test failed"

In the no-save mode, you can see differences, but no merging is enabled (kdiff3 used in diff-view mode only).

xkrogen commented 9 years ago

Why is it set up within bond_reconcile such that the diff isn't printed if the test failed? If you want it that way so that you just print a log of the observations thus far instead of a diff that's fine, but right now the message is misleading:

bond_reconcile.py:238 prints "These are the differences for {}", but doesn't actually print any differences

On Sat, Oct 17, 2015 at 1:19 PM, George Necula notifications@github.com wrote:

Assigned #14 https://github.com/necula01/bond/issues/14 to @xkrogen https://github.com/xkrogen.

— Reply to this email directly or view it on GitHub https://github.com/necula01/bond/issues/14#event-438314969.

xkrogen commented 9 years ago

Actually, I see now that you can use [d]iff to show the diff. The message is still confusing, though.

On Mon, Oct 19, 2015 at 1:54 PM, Erik Krogen erikkrogen@gmail.com wrote:

Why is it set up within bond_reconcile such that the diff isn't printed if the test failed? If you want it that way so that you just print a log of the observations thus far instead of a diff that's fine, but right now the message is misleading:

bond_reconcile.py:238 prints "These are the differences for {}", but doesn't actually print any differences

On Sat, Oct 17, 2015 at 1:19 PM, George Necula notifications@github.com wrote:

Assigned #14 https://github.com/necula01/bond/issues/14 to @xkrogen https://github.com/xkrogen.

— Reply to this email directly or view it on GitHub https://github.com/necula01/bond/issues/14#event-438314969.

xkrogen commented 9 years ago

Fixed in rbond with above commit. @necula01 what do you want to do about the message? I was thinking of just changing it to: "Observations are shown for {}. Saving them not allowed. Use the diff option to show the differences." or something along those lines.

gnecula commented 9 years ago

I agree that the message is confusing, but when working with it I have discovered that I actually want to see the error message first, and then, maybe, see the diff.

George.

On Mon, Oct 19, 2015 at 1:55 PM, Erik notifications@github.com wrote:

Actually, I see now that you can use [d]iff to show the diff. The message is still confusing, though.

On Mon, Oct 19, 2015 at 1:54 PM, Erik Krogen erikkrogen@gmail.com wrote:

Why is it set up within bond_reconcile such that the diff isn't printed if the test failed? If you want it that way so that you just print a log of the observations thus far instead of a diff that's fine, but right now the message is misleading:

bond_reconcile.py:238 prints "These are the differences for {}", but doesn't actually print any differences

On Sat, Oct 17, 2015 at 1:19 PM, George Necula <notifications@github.com

wrote:

Assigned #14 https://github.com/necula01/bond/issues/14 to @xkrogen https://github.com/xkrogen.

— Reply to this email directly or view it on GitHub https://github.com/necula01/bond/issues/14#event-438314969.

— Reply to this email directly or view it on GitHub https://github.com/necula01/bond/issues/14#issuecomment-149344415.

gnecula commented 9 years ago

I think that your proposal is fine.

George.

On Mon, Oct 19, 2015 at 2:10 PM, Erik notifications@github.com wrote:

Fixed in rbond with above commit. @necula01 https://github.com/necula01 what do you want to do about the message? I was thinking of just changing it to: "Observations are shown for {}. Saving them not allowed. Use the diff option to show the differences." or something along those lines.

— Reply to this email directly or view it on GitHub https://github.com/necula01/bond/issues/14#issuecomment-149348630.

xkrogen commented 9 years ago

Updated to use the message I proposed