Rename BaseEngineParTest to BaseEngineTest

[Anmol-Singh-Jaggi]

This change resolves #127

[Anmol-Singh-Jaggi]

@junchuanwang Any updates on this?

[junchuanwang]

Hi in this PR, you should keep the consumer of BaseEngineTest the same to test if it is backward compatible, because you are renaming BaseEngineTest

Do you think you can do that and test it? @Anmol-Singh-Jaggi

[junchuanwang]

I think this is backward-incompatible change, isn't it?

Any existing user of BaseEngineTest if keep using BaseEngineTest, will potentially fail if their test cannot be run in multi-htreaded environment. @Anmol-Singh-Jaggi

[Anmol-Singh-Jaggi]

Whatever test was running fine as BaseEngineTest should run fine as BaseEngineParTest as per my understanding.
BaseEngineParTest just uses one engine for all test methods.
Whereas BaseEngineTest creates a new engine for every test method.

This change will be incompatible only when a test relies on a fresh engine before every test method. However I don't see why anyone would write such a test IMHO.

Also, I noticed that BaseEngineTest earlier had methods setUpBaseEngineTest(), setUp(), tearDown(), tearDownBaseEngineTest() but after this change it will have methods setUpBaseEngineParTest() and tearDownBaseEngineParTest().
This is backwards-incompatible. I forgot to rename the new methods back to how they were before; will fix this.

PS: In Linkedin codebase, in many places setUpBaseEngineTest() and tearDownBaseEngineTest() are being incorrectly used. They are being overriden to initialized mocks and stubs. More importantly, in the overriden method body, the super method is not being called. As a result, the parseq engine should not have been initialized/destroyed before/after every method theoretically.
However, this is inadvertently being prevented by the other alias methods setUp() and tearDown().
Basically, TestNg executes both setUpBaseEngineTest() and setUp() before every method since they are both marked @BeforeMethod.
Now when setUpBaseEngineTest() is overriden just to initialize mocks and super is not called, the engine is instead initialized by the setUp() method. This is working for now, but when setUp() is removed in the next major version, a lot of these tests will break since parseq engine would not have been initialized (I understand that this is the client's fault since they are not using inheritance correctly, and not parseq's fault).

For example, you can look at the following files in the Linkedin codebase:

• FeatureControlSettingsServiceTest and SessionTimeoutHandlerTest in the mp learning-enterprise-api.
• CommentWorkflowTest in the mp comment-ranker.
• AdReviewDataManagerTest in the mp tscp-manual-review.

super() is being called correctly in very few case, like LearnerCertificateMetricPinotDataExtractorTest in the mp learning-common.

[Anmol-Singh-Jaggi]

"you should keep the consumer of BaseEngineTest the same to test if it is backward compatible, because you are renaming BaseEngineTest"

Good idea; will also make this change.