Thanks very much, I'm now able to write a useful unit test to catch the expected exception. Given the great support I've had from the list, I'll start my organisation's process to contribute the code back for this:
https://jira.apache.org/jira/browse/NIFI-5538 On Fri, 24 Aug 2018 at 16:41, Mark Payne <marka...@hotmail.com> wrote: > > James, > > Yes, makes perfect sense. I think that's a good balance of logic, as well. > Use a validator to quickly > ensure that the file exists. Then, when trying to use it, go ahead and parse > the data and set your > member variable. You could have the validator parse the file (but not set the > member variable) to > ensure that the format is valid, etc. but I would personally avoid doing > that, because the parsing > may well prove to be quite expensive for validation purposes. I think you're > very much on the right track. > > Thanks! > -Mark > > > > > On Aug 24, 2018, at 11:34 AM, James Srinivasan <james.sriniva...@gmail.com> > > wrote: > > > > Mark, > > > > Thanks very much for the detailed answer. In my particular case, I > > have a parameter corresponding to a schema file on disk and there is a > > standard validator to ensure that the file exists. Currently, in my > > @OnScheduled method, I read that schema file, parse it and store the > > parsed results in a member of my class ready for use in my onTrigger > > method. If the file fails to parse, an exception is thrown. I could > > move that code into a validator, but setting a member as a side effect > > of validation didn't quite feel right - does that makes sense? > > > > James > > On Fri, 24 Aug 2018 at 16:01, Mark Payne <marka...@hotmail.com> wrote: > >> > >> James, > >> > >> You can certainly catch Throwable there, or AssertionError, more > >> specifically, but I'd be very wary > >> of doing that, because at that point you're really kind of working against > >> the framework (both the > >> nifi mock/test framework as well as the JUnit framework) instead of with > >> it. If your intent is to test > >> a specific method, I would recommend testing that method explicitly by > >> calling it yourself. > >> > >> You don't need to create your own MockProcessContext. You can get the > >> ProcessContext from > >> the Test Runner. For example: > >> > >> > >> final MyProcessor myProcessor = new MyProcessor(); > >> final TestRunner runner = TestRunners.newTestRunner(myProcessor); > >> > >> runner.setProperty(MyProcessor.MY_PROPERTY, "hello"); > >> > >> try { > >> myProcessor.onScheduled( runner.getProcessContext() ); > >> Assert.fail("Expected SomeException to get thrown from onScheduled method > >> but it did not."); > >> } catch (final SomeException e) { > >> // expected. > >> } > >> > >> Now, this being said, it begs the question whether or not you want to be > >> throwing an Exception from your @OnScheduled method. > >> I'm sure there are use cases where this makes perfect sense. However, you > >> should first think about whether or not you are > >> able to prevent the Exception from occurring by applying validation rules > >> (addValidator() to PropertyDescriptor's or customValidate). > >> The benefit to validators here is that when the user configures the > >> Processor incorrectly, they get a clear indication immediately that it > >> is not valid and a clear explanation of why it's not valid (as well as > >> being shown in the Invalid Counts of Process Groups, etc.). > >> If you wait until the user tries to start the Processor and throw an > >> Exception, it will be less obvious that there's a configuration problem > >> and the error message that they receive is likely not to be as clear. > >> > >> Thanks > >> -Mark > >> > >> > >> On Aug 23, 2018, at 5:25 PM, James Srinivasan > >> <james.sriniva...@gmail.com<mailto:james.sriniva...@gmail.com>> wrote: > >> > >> Ah, hadn't spotted that. It's close, but the Throwable I get is a > >> java.lang.AssertionError (Could not invoke methods annotated with > >> @OnScheduled annotation due to: > >> java.lang.reflect.InvocationTargetException) and there doesn't seem to > >> be any way to get the actual underlying exception my code threw in > >> order to properly validate it. > >> > >> Mark's suggestion of calling the @OnScheduled method directly seems a > >> little tricky when using the TestRunner framework, or should I just > >> replicate the test setup (e.g. create my own MockProcessContext etc.) > >> > >> Thanks, > >> > >> James > >> On Thu, 23 Aug 2018 at 21:03, Mike Thomsen > >> <mikerthom...@gmail.com<mailto:mikerthom...@gmail.com>> wrote: > >> > >> James try it with a throwable like in my example > >> On Thu, Aug 23, 2018 at 10:51 AM Mark Payne > >> <marka...@hotmail.com<mailto:marka...@hotmail.com>> wrote: > >> > >> James, > >> > >> If you are expecting the method to throw an Exception and want to verify > >> that, you should > >> just call the method directly from your unit test and catch the Exception > >> there. The TestRunner > >> expects to run the full lifecycle of the Processor. > >> > >> Thanks > >> -Mark > >> > >> > >> On Aug 23, 2018, at 10:49 AM, James Srinivasan < > >> james.sriniva...@gmail.com<mailto:james.sriniva...@gmail.com>> wrote: > >> > >> I tried that, but the problem is the exception is caught and the test > >> fails due to this: > >> > >> try { > >> ReflectionUtils.invokeMethodsWithAnnotation(OnScheduled.class, > >> processor, context); > >> } catch (final Exception e) { > >> e.printStackTrace(); > >> Assert.fail("Could not invoke methods annotated with @OnScheduled > >> annotation due to: " + e); > >> } > >> > >> > >> https://github.com/apache/nifi/blob/master/nifi-mock/src/main/java/org/apache/nifi/util/StandardProcessorTestRunner.java#L181 > >> On Thu, 23 Aug 2018 at 15:41, Mike Thomsen <mikerthom...@gmail.com> > >> wrote: > >> > >> For unit tests, if you're doing this to catch a failure scenario, you > >> should be able to wrap the failing call in something like this: > >> > >> final def msg = "Lorem ipsum..." > >> def error = null > >> try { > >> runner.run() > >> } catch (Throwable t) { > >> error = t > >> } finally { > >> assertNotNull(error) > >> assertTrue(error.cause instanceof SomeException) > >> assertTrue(error.cause.message.contains(msg)) > >> } > >> > >> Obviously play around with the finally block, but I've had success with > >> that pattern. > >> > >> On Thu, Aug 23, 2018 at 10:19 AM James Srinivasan < > >> james.sriniva...@gmail.com> wrote: > >> > >> What is the best way to handle exceptions which might be thrown in my > >> @OnScheduled method? Right now, I'm logging and propagating the > >> exception which has the desired behaviour in NiFi (bulletin in GUI and > >> processor cannot be started) but when trying to add a unit test, the > >> (expected) exception is caught in StandardProcessorTestRunner.run and > >> failure asserted. > >> > >> My actual @OnScheduled method builds a non-trivial object based on the > >> Processor's params - maybe I should be building that any time any of > >> the params change instead? > >> > >> Many thanks, > >> > >> James > >> > >> > >> > >> >
