Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349209 - [EUnit] Arguments of assert* (except assertError) are evaluated twice
Summary: [EUnit] Arguments of assert* (except assertError) are evaluated twice
Status: CLOSED FIXED
Alias: None
Product: Epsilon
Classification: Modeling
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Antonio Garcia-Dominguez CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-13 12:33 EDT by Antonio Garcia-Dominguez CLA
Modified: 2012-02-06 10:59 EST (History)
2 users (show)

See Also:


Attachments
EOL script which reproduces the problem (requires SVN r1520 or later) (681 bytes, text/plain)
2011-06-13 12:34 EDT, Antonio Garcia-Dominguez CLA
no flags Details
Patch which fixes the issue, assuming all built-in operations are not overridable (1.69 KB, patch)
2011-06-13 12:48 EDT, Antonio Garcia-Dominguez CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Garcia-Dominguez CLA 2011-06-13 12:33:44 EDT
I have found out that, apparently, every argument for an assert* operation (such as assertTrue) is evaluated twice. This is a performance problem, and may produce incorrect results if any of the repeatedly evaluated expressions has side effects.

I have attached an EOL script which reproduces the problem on Epsilon SVN (r1520 and beyond, as it depends on the fix for bug #349207). Both assertions in this script should pass. However, the second assertion fails:

start with seq1
first time: setting to 1
end with seq1
start with seq2
first time: setting to 1
incremented to 2
Expected 1, but got 2 instead (/home/antonio/Documentos/sodmt/workspace-metamodels/EOLScratchpad/testDoubleEvaluation.eol@12:0)

After looking at the code, it seems to be caused by the way method lookups are performed in PointExecutor#execute:

- non-overrdable operations (assertError seems to be the only one) are run
- parameters are evaluated
- find and run user-defined operation if applicable
- find and run method contributor if applicable
- overridable operations (such as assertEquals or assertTrue)

Since those overridable operations evaluate the parameters once more, every parameter will be evaluated twice.

I'm not quite sure about what would be the best way to fix this. Some operations (such as select or aggregate) do need specific code to parse their internal ASTs, but most AbstractSimpleOperations do not. We could add a special case for overridable AbstractSimpleOperations, but that sounds like a hack and not like a general solution.
Comment 1 Antonio Garcia-Dominguez CLA 2011-06-13 12:34:27 EDT
Created attachment 197900 [details]
EOL script which reproduces the problem (requires SVN r1520 or later)
Comment 2 Antonio Garcia-Dominguez CLA 2011-06-13 12:48:10 EDT
Created attachment 197902 [details]
Patch which fixes the issue, assuming all built-in operations are not overridable

For the time being, I'm working around the issue by assuming none of these operations can be overridden. Here is the patch I'm using.
Comment 3 Antonio Garcia-Dominguez CLA 2011-07-06 05:26:56 EDT
We fixed this on SVN on our last meeting. Since then, I have moved the EUnit assertions to three new operation contributors:

- a basic contributor with all assertions that are simple and do not require additional dependencies (in eol.engine).

- an extra contributor with assertions that are more complex and/or require additional dependencies (in eunit.engine).

- a contributor which is added from the EUnit Ant task, as the contributed operations require a reference to the Ant Project instance.
Comment 4 Dimitris Kolovos CLA 2011-07-25 08:18:35 EDT
Fixed in 0.9.1