Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[junit5] Extensions to properly handle parallelism #114

Closed
kriegfrj opened this issue May 18, 2020 · 1 comment
Closed

[junit5] Extensions to properly handle parallelism #114

kriegfrj opened this issue May 18, 2020 · 1 comment
Milestone

Comments

@kriegfrj
Copy link
Contributor

While having a look at #113, I realised that our current implementation of the JUnit 5 extensions and use of the Store is not parallel-safe. Because neither the key nor the namespace that we are using for the context lookup makes reference to the test method, every test method within the test class will always get the same BundleContext instance (or whatever state object it is storing). If their execution happens to overlap, this will cause them to interfere with each other.

The fix should be simple - to include the method name in the Namespace. See the TimingExtension.getStore() example in the JUnit 5 user doc here

I will look at this in conjunction with #113.

@kriegfrj
Copy link
Contributor Author

When I implemented my first set of test cases to test for this condition, I was surprised to find that they passed. On reflection, this makes sense as every method already gets its own unique ExtensionContext.

However, it might become an issue in conjunction with #113, because of the hierarchical lookup - if the Store doesn't find its key in the current context it will look in the parent context, and if it doesn't find one there only then it will create a new one. Our implementation currently works because we never create our state object at the level of the test class, so the parent context never has one. But if I implement #113 then this might no longer apply.

It also occurred to me that in the current implementation, the bundle context state may persist between dynamic tests, because each dynamic test is the child of a test method, and so every dynamic invocation's context lookup will retrieve the parent test method's context. I haven't tested this theory yet but will shortly.

kriegfrj added a commit to kriegfrj/osgi-test that referenced this issue Jun 3, 2020
Fixes osgi#113, fixes osgi#114

Also improved test fidelity by using EngineTestKit

Fixes osgi#116.

Signed-off-by: Fr Jeremy Krieg <fr.jkrieg@greekwelfaresa.org.au>
@bjhargrave bjhargrave added this to the 1.0.0 milestone Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants