Identify ways of reducing the memory foot print of ITestResult

[krmahadevan]

Currently every ITestResult object holds reference to the below 3 things.

1. A reference to the java.lang.reflect.Method object.
2. A reference to the Throwable object for failed tests.
3. A reference to the actual instance associated with a particular test method (this instance reference could be shared across test methods, if there were more than 1 test method that belonged to the same instance)
4. A reference to the array of parameters that were associated with a test method (if the test method was either data driven or parameterized via the @Parameters annotation.

For small test suites this doesn't pose a big problem. But as the size of a test suite starts increasing this basically starts bloating the memory and could result in OutOfMemoryError causing a test run to break without completing the execution.

This issue is to facilitate brainstorming on the ways in which we could think of achieving this.

cc @cbeust @juherr

[krmahadevan]

Thinking aloud here

As a first iteration I was thinking of breaking the link between an ITestResult object and its corresponding ITestNGMethod object.

ITestNGMethod interface exposes the following objects (which kind of brings in everything)

1 org.testng.ITestNGMethod#getTestClass and via the ITestClass we expose an array of ITestNGMethod objects which represent test and configuration methods in a given class. 2 org.testng.ITestNGMethod#getConstructorOrMethod which internally represents the java.lang.reflect.Method object.

IMO, both the above two things should be decoupled/removed from the reach of an ITestResult object, because I am not sure what would a user be doing with these objects. So we could think of delinking these information from an ITestResult object after a test result has been calculated and all its respective listeners (applicable to their current state) have been invoked.

From a test result's perspective, all a user would need to know is the following

• The method to which the current test result belongs to (This could be a new object that adheres to the ITestNGMethod interface but is not aware of (1) and (2) and the meta data of that actual method (perhaps things such as the list of annotations added to that method, incase a user would like to enrich their custom report with this data)

This would be a backward compatibility breaking change, but I guess we should be fine since we are enroute to doing a major release.

@juherr @cbeust - Please let me know your thoughts on this.

[juherr]

I'd prefer to find a not breaking change solution. I was thinking about an event-driven architecture where the default implementation will construct the current model. I can imagine some reporters able to report in realtime but some others not.

But I didn't find the best strategy to minimize the internal modifications because the model is deeply linked into the core.

[kool79]

For me PR #1978 (Nullify Throwable for passed tests) will break my detailed test report.

One of my customers requires such detailed reporting for sensitive appliations. Those reports usually reviewed and analyzed by separate team. They wants to see every verification step (including passed ones) to catch false-positive, never-failing tests (without asserts) or tests with unvalid expected data.

I propose to make this "nullify" optional or use approach which was proposed by author of #1882 in second comment: set throwable to null with interceptor. It effectively solves his issue without limit other TestNG functionality. (Maybe we need some guide 'How To Reduce Memmory Footprint' where this will de described.)

In any case this PR will not solve memmory issue for huge suites (those who has 100k testcases in one suite will not stop to add new ones :) ). While there are many ways to solve this particular issue: use interceptor, split suite, increase memmory etc

[krmahadevan]

@juherr - I am not too keen on introducing a configuration parameter just for this. What do you suggest ? Do we revert back the PR and just advise the user to take the listener route and manage the memory bloat at their test project itself ?

[krmahadevan]

@kool79

In any case this PR will not solve memory issue for huge suites

This PR was never meant to tackle memory issues for sure. At least I didn't have that in mind. It was just a fix for a bug that was raised. Nothing more nothing less.

I wrote up this bug so that it serves as a mechanism to capture various different ideas on how we could effectively slim down the test result (didn't want the discussion to be scattered all over the place). If you have ideas on how it can be accomplished, please do feel free to chime in.

[krmahadevan]

@juherr

I'd prefer to find a not breaking change solution. I was thinking about an event-driven architecture where the default implementation will construct the current model.

Me too. The only way out would be to rethink on the reporting part, which perhaps means that people would have to perhaps rebuild their entire reporting mechanisms.

I guess the main issue right now is that we expose too much via the List<ISuite> in the reporting phase. We may have to think of perhaps introducing a new method within the IReporter interface that exposes a basic ISuite variant with only access to the test results and only enough information about the method to which it belongs to, the annotations on that method and the parameters used etc., That would again mean the same thing. TestNG users would have to rebuild their reporting mechanism once again (I guess that would be a setback in terms of adopting the newer version).

[juherr]

@krmahadevan As the PR is breaking production things, you can revert it or make it configurable. Both are good IMO.

About a new reporter model, the more difficult part will be to find a way to refactor and keep working the old fashion model.

[krmahadevan]

@juherr - I reverted the changes.