Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expect.equal throws an NRE on failure #330

Closed
isaacabraham opened this issue Jun 30, 2019 · 12 comments
Closed

Expect.equal throws an NRE on failure #330

isaacabraham opened this issue Jun 30, 2019 · 12 comments

Comments

@isaacabraham
Copy link
Contributor

On Expecto 8.10.1, the following code in an F# script fails with an NRE rather than a "pretty" error.

open Expecto
Expect.equal 1 2 "Doh!"

with the error:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Expecto.Expect.highlightAllGreen(FSharpList`1 diffs, String s) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Expect.fs:line 45
   at Expecto.Expect.printVerses[a,b](String firstName, a first, String secondName, b second) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Expect.fs:line 66
   at Expecto.Expect.equal$cont@317[a](a actual, a expected, String message, Object e, Object a, Unit unitVar) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Expect.fs:line 335
   at <StartupCode$FSI_0014>.$FSI_0014.main@()
Stopped due to error

Is this something I'm doing "wrong" - can you not call Expect. functions directly in a script or something?

@isaacabraham
Copy link
Contributor Author

Seems that this error goes away if you call runTests on some arbitrary set of tests first, which suggests that some shared static state first needs to be set before you can run Expect. functions. Is there a way to avoid this? It's surprising to couple a test runner to individual predicate / test helpers.

@AnthonyLloyd
Copy link
Contributor

It's a colours config that is used in the error messages. I've defaulted to 0 if no config sets it. Will go out soon. Workaround is to runTests on an empty set of tests before any Expect.

@AnthonyLloyd
Copy link
Contributor

@isaacabraham can I just check what you are looking to use this for. Just want to consider the default colours. If it's in another test runner I'm not sure if colours would be useful.

haf added a commit that referenced this issue Jul 2, 2019
@isaacabraham
Copy link
Contributor Author

@AnthonyLloyd Yes, I just opened the REPL and wanted to experiment with the testing library behaviours outside the context of the test runner.

@isaacabraham
Copy link
Contributor Author

isaacabraham commented Jul 2, 2019

@AnthonyLloyd additionally, when running in the context of e.g. azure dev ops, a load of control codes seem to be emitted in the console and get in the way of the real textual output. However, that's a side issue related to colours rather than this NRE.

@isaacabraham
Copy link
Contributor Author

I suspect that the commit linked above (5a675cb) won't address the root cause e.g. someone running a script on mac or linux will still get this error.

@AnthonyLloyd
Copy link
Contributor

@isaacabraham Need to set --colours 0. They are ANSI control codes. No sensible way of detecting if they are not supported and we default to 8 colours.

@AnthonyLloyd
Copy link
Contributor

I suspect that the commit linked above (5a675cb) won't address the root cause e.g. someone running a script on mac or linux will still get this error.

They'll get the ANSI colour codes.

If you know of any library that handles colour well please let me know. Note we moved from Console commands to ANSI to make it atomic and stop the bug often seen with colour being incorrectly reset.

@haf
Copy link
Owner

haf commented Jul 2, 2019

@AnthonyLloyd What would you have against checking whether the terminal is interative? #315 (comment) — if you have a terminal like Travis CI that supports ANSI escape characters, they would be correctly parsed, or if you have Azure, then presumable tty would be false. Would perhaps @isaacabraham be willing to run a script on Azure to test whether this assumption is true?

@isaacabraham
Copy link
Contributor Author

I think we're conflating the issue here. That was simply about getting an NRE if you call Expecto functions outside of the test runner. Putting a check for Windows won't help that at all I would imagine.

But of course happy to test with dev ops. But sounds like the color argument above will do the trick - I'll try that out

@AnthonyLloyd
Copy link
Contributor

@haf Nothing against it if we can be sure it's a solution. Just wary of complexity that's hard to test.

@isaacabraham sorry, cross issues colours discussion.

@haf
Copy link
Owner

haf commented Jul 2, 2019

Putting a check for Windows won't help that at all I would imagine.

No, that's separate; the change that fixed your problem was to default the colours setting to something other than null (None).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants