Bug 61534 - Convert AssertionError to a failed assertion in the JSR223Assertion allowing users to use assert in their code
Summary: Convert AssertionError to a failed assertion in the JSR223Assertion allowing ...
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 3.2
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2017-09-18 19:40 UTC by Felix Schumacher
Modified: 2018-01-25 21:24 UTC (History)
2 users (show)



Attachments
Catch AssertionError on assertions and use them to fill in the failure message (1.39 KB, patch)
2017-09-18 19:40 UTC, Felix Schumacher
Details | Diff
Catch AssertionError on assertions and use them to fill in the failure message (13.68 KB, patch)
2018-01-19 11:26 UTC, Felix Schumacher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Schumacher 2017-09-18 19:40:37 UTC
Created attachment 35333 [details]
Catch AssertionError on assertions and use them to fill in the failure message

After seeing Philippes commits for JDBC tests that used groovy for assertions. I thought how much shorter those assertions could be, if he could have written:

def list = vars.getObject('result')
assert list.size() == 1
assert list[0]['AUTHOR'] == '...'

instead of the current version

def list = vars.getObject('result');
if (list.size()==1) {
       def map = list.get(0);
       if(map.get('AUTHOR').equals('Philip K. Dick')) {
               AssertionResult.setFailure(false);
       } else {
               AssertionResult.setFailure(true);
               AssertionResult.setFailureMessage("Expected first row AUTHOR to be equal to 'Philip K. Dick'");
       }
} else {
       AssertionResult.setFailure(true);
       AssertionResult.setFailureMessage('Expected 1 row in result, got:'+list.size());
}
Comment 1 Philippe Mouawad 2017-09-18 19:45:09 UTC
Great idea !
Comment 2 Philippe Mouawad 2017-10-23 12:08:42 UTC
Author: pmouawad
Date: Mon Oct 23 12:08:24 2017
New Revision: 1813001

URL: http://svn.apache.org/viewvc?rev=1813001&view=rev
Log:
Bug 61534 - Convert AssertionError to a failed assertion in the JSR223Assertion allowing users to use assert in their code
Patch by Felix Schumacher
Bugzilla Id: 61534

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/assertions/JSR223Assertion.java
    jmeter/trunk/xdocs/changes.xml
Comment 3 Felix Schumacher 2018-01-07 14:03:02 UTC
This "fix" was needed, as a result of r1775911, which changed a catch clause in JMeterThread#processAssertion from Error to JMeterError. Therefore this patch brings back part of the behaviour from 3.1 back.

I wonder if we should revert or at least extend the catch clause in JMeterThread#processAssertion instead of JSR223Assertion, only.
Comment 4 Philippe Mouawad 2018-01-07 14:04:26 UTC
Hi Felix,
I think we should restore the 3.1 behaviour 
Regards
Comment 5 Philippe Mouawad 2018-01-07 14:04:58 UTC
(In reply to Philippe Mouawad from comment #4)
> Hi Felix,
> I think we should restore the 3.1 behaviour 
as you proposed. 
I have seen on SO a person complaining about this.
> Regards
Comment 6 Felix Schumacher 2018-01-07 14:29:31 UTC
(In reply to Philippe Mouawad from comment #4)
> Hi Felix,
> I think we should restore the 3.1 behaviour 
> Regards

I tend to add AssertionError to JMeterError, as normal programs should not catch Error (most are low level errors we can't handle correctly). That way, we could get rid of catching ThreadDeath, too.
Comment 7 Felix Schumacher 2018-01-19 11:21:46 UTC
One other thing, or rather two.

If we add AssertionError to the JMeterError catch clause, the failed assertions will be marked as error instead of failure and (more importantly) log stacktraces to jmeter.log.

So my questions are:

Should we log those assertion failures at all, or at debug level, only?

Is an AssertionError an error or a failure in respect to Assertions?
Comment 8 Felix Schumacher 2018-01-19 11:26:28 UTC
Created attachment 35685 [details]
Catch AssertionError on assertions and use them to fill in the failure message

This patch reverts the previous one and places the logic to catch AssertionErrors higher up in the stack.

This fixes an regression introduced by r1775911 and reduces the noise in jmeter.log introduced by thoses AssertionErrors.
Comment 9 Felix Schumacher 2018-01-25 21:24:40 UTC
Will be included in 4.0.

Date: Thu Jan 25 21:22:58 2018
New Revision: 1822229

URL: http://svn.apache.org/viewvc?rev=1822229&view=rev
Log:
Convert AssertionError to a failed assertion for all kind of assertions.

Fixing a regression introduced in 3.2. ThreadDeath has not to be catched anymore,
as we are not using Error in the catch clause (which was the case before r1775911
that introduced this regression).

Bugzilla Id: 61534

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/assertions/JSR223Assertion.java
    jmeter/trunk/src/core/org/apache/jmeter/threads/JMeterThread.java
    jmeter/trunk/xdocs/changes.xml