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

Some managers like ContentModelManager, URIResolverManagerExtension should not be a singleton #215

Closed
angelozerr opened this issue Nov 12, 2018 · 3 comments
Assignees
Labels
bug Something isn't working debt This issue or enhancement is related to technical debt
Milestone

Comments

@angelozerr
Copy link
Contributor

When I try to implement #204 by adding a new simple URIResolver https://github.com/angelozerr/lsp4xml/blob/master/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/catalog/XMLCatalogURIResolverExtension.java it breaks tests. It's so crazy:

  • when I launch the all tests, some tests failed.
  • when I launch only a test which has failed, it works?

After spending a lot of time, I see the problem is because each time test is executed, it updates singleton with some uriresolver, and we are not in the same context if we launch the all tests and just one test.

The problem is because URIResolverManagerExtension is a singleton, although it should be a singleton only for an instance of XMLLanguageService.

This issue is for link an instance of URIResolverManagerExtension to an instance of XMLLanguageService to be in the same context (in test context) than real case

I'm working on this issue.

@angelozerr angelozerr added the bug Something isn't working label Nov 12, 2018
@angelozerr angelozerr added this to the v0.0.3 milestone Nov 12, 2018
@angelozerr angelozerr self-assigned this Nov 12, 2018
angelozerr added a commit that referenced this issue Nov 12, 2018
@angelozerr
Copy link
Contributor Author

This issue should be fixed. There were several JUnit tests which was wrong because they had some dependencies from a previous test (ex: a test which fill content model manager, and after the test used the filled content model manager).

Now we have clean tests.

@fbricon fbricon changed the title Some manage like ContentModelManager, URIResolverManagerExtension should not be a singleton Some managers like ContentModelManager, URIResolverManagerExtension should not be a singleton Nov 12, 2018
fbricon added a commit that referenced this issue Nov 12, 2018
@fbricon
Copy link
Contributor

fbricon commented Nov 12, 2018

I've enabled runorder=random in the maven surefire plugin configuration, so we can better catch similar test failures. See 2b80639

@fbricon fbricon added the debt This issue or enhancement is related to technical debt label Nov 12, 2018
@angelozerr
Copy link
Contributor Author

I've enabled runorder=random

I didn't know that. Thank @fbricon !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working debt This issue or enhancement is related to technical debt
Projects
None yet
Development

No branches or pull requests

2 participants