Adding a test for the null pointer exception I ran across today.

[berlin-ab]

Should the constructor take another argument, or should the constructor
be more defensive?

[Crisfole]

Hey Adam, was this NPE from having an null in the custom field? I have a fix for that in place.

My goal was to have all constructors work as is...I'll go test that locally.

[berlin-ab]

Yep, that's the one. We also found another NPE when calling #log. I can file a separate test for that if you'd like.

[Crisfole]

You can just use fail() here, and then drop the exceptionDidHappen field.

Also you might be better off with:

Rollbar r = new Rollbar("Some access token", "environment name");
pass();

The uncaught error will propagate up and will display better in the test panels/command line. (Because it will include the exception stack trace).

[Crisfole]

@berlin-ab Please do file the other test! This is a new library that I'm really excited about, but it needs some battle hardening.

[berlin-ab]

I wanted to make the intention of the test clear and I wasn't sure that raising the exception would do that. I'll try out fail() and see how I feel about it.

[berlin-ab]

Thanks for making this library. I've used Rollbar on other projects in different languages, and I was disappointed to not find something more stable for Java. I'm happy to help.

[Crisfole]

@berlin-ab I've also got some hamcrest test utilities already present. You can probably find something like 'doesNotThrow' (maybe).

[Crisfole]

Better yet, you could probably just create a testContructorWorks test:

Rollbar r = new Rollbar("at", "env");
assertEqual("at", r.accessToken());
assertEqual("env", r.environment());

To explain my hesitation: By having sufficient coverage (and ensuring we've run through at least all the functions/constructors) we can get a full stack trace for any uncaught exceptions, without ever having to explicitly test that a particular call doesn't throw.

Make sense?

[berlin-ab]

Ok, updated with production code.

Side note, I'm not exactly sure what custom means for this variable. Can you think of another name that is more descriptive?

[Crisfole]

In this case the 'defaultCustom' would actually need to be null, which would cause it to be skipped in the serializer.

[Crisfole]

In this case there's literally nothing more descriptive since it contains all the "custom" data you want to send. Literally whatever you want.

Grep for "custom" in here: https://rollbar.com/docs/api/items_post/

[berlin-ab]

Ok, I've pushed a new commit that allows custom to be nil and just calls the copy constructor.

[Crisfole]

You rock. I'm going to pull this, run the tests, and merge it in, thanks.

[Crisfole]

And then I'm going to hook up travis...

[berlin-ab]

Great, thanks!