gocheck

Merge lp://qastaging/~rogpeppe/gocheck/error-fixes into lp://qastaging/gocheck

error-fixes
Merge into trunk

Proposed by Roger Peppe on 2011-11-08

Status:	Merged
Merged at revision:	62
Proposed branch:	lp://qastaging/~rogpeppe/gocheck/error-fixes
Merge into:	lp://qastaging/gocheck
Diff against target:	1088 lines (+161/-147) 13 files modified bootstrap_test.go (+0/-2) checkers.go (+68/-25) checkers_test.go (+72/-39) export_test.go (+1/-5) fixture_test.go (+0/-11) foundation_test.go (+0/-3) gocheck.go (+11/-19) gocheck_test.go (+1/-10) helpers.go (+0/-4) helpers_test.go (+3/-13) printer.go (+1/-1) run.go (+1/-4) run_test.go (+3/-11)
To merge this branch:	bzr merge lp://qastaging/~rogpeppe/gocheck/error-fixes
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gustavo Niemeyer		2011-11-08	Approve on 2011-11-09
Review via email: mp+81599@code.qastaging.launchpad.net

Description of the change

gocheck fix up for new error type.

make errors print their strings too.
stringer is a more convention name than hasString.

Revision history for this message

Gustavo Niemeyer (niemeyer) wrote on 2011-11-09:

[1]

+// c.Assert(err, Matches, "perm.*denied")

s/Matches/ErrorMatches/

[2]

+// The Panics checker verifies that calling the provided zero-argument

s/Panics/PanicMatches/

[3]

199 + if obtained == nil {
200 + error = "Function has not panicked"
201 + return
202 + }
(...)
214 - return false, "Function has not panicked"

Please preserve the original semantics here. It is possible for a function
to panic with a nil value, and the original way the "function has not panicked"
check worked would inform the developer of the proper problem, while the
current one will pretend the panic didn't happen.

Can you please add a test for that case too?

review: Needs Fixing

Revision history for this message

Roger Peppe (rogpeppe) wrote on 2011-11-09:

On 9 November 2011 13:30, Gustavo Niemeyer <email address hidden> wrote:
> Review: Needs Fixing
>
> [1]
>
> +// c.Assert(err, Matches, "perm.*denied")
>
> s/Matches/ErrorMatches/

done

>
> [2]
>
> +// The Panics checker verifies that calling the provided zero-argument
>
> s/Panics/PanicMatches/

done

>
> [3]
>
> 199 + if obtained == nil {
> 200 + error = "Function has not panicked"
> 201 + return
> 202 + }
> (...)
> 214 - return false, "Function has not panicked"
>
> Please preserve the original semantics here. It is possible for a function
> to panic with a nil value, and the original way the "function has not panicked"
> check worked would inform the developer of the proper problem, while the
> current one will pretend the panic didn't happen.

done

>
> Can you please add a test for that case too?

done

lp://qastaging/~rogpeppe/gocheck/error-fixes updated on 2011-11-09