Virtual pointer hasn't been disposed on LATEST-EAP-SNAPSHOT

Hi,

 

I noticed that the tests for my plugin are failing on LATEST-EAP-SNAPSHOT due to "TraceableDisposable$DisposalException: Virtual pointer '' hasn't been disposed". 

 

You can see the failing build here: https://travis-ci.org/mapstruct/mapstruct-idea/builds/290783008. I am not sure if I have been using something wrong, there has been some change in the LATEST-EAP-SNAPSHOT that I have missed or is some bug in the release.

 

I would appreciate the help.

Filip

12 comments
Comment actions Permalink
Official comment

The tests in our test framework now have a policy "No leaking libraries and SDKs". That means that when you create a library (e.g. in MapstructBaseCompletionTestCase.java:57) there must be a corresponding removeLibrary(), e.g. in your tearDown().

You can also use com.intellij.testFramework.PsiTestUtil#addLibrary(com.intellij.openapi.Disposable, com.intellij.openapi.module.Module, java.lang.String, java.lang.String, java.lang.String...) for adding the library instead of com.intellij.testFramework.PsiTestUtil#addLibrary(com.intellij.openapi.module.Module, java.lang.String, java.lang.String, java.lang.String...). The former accepts Disposable which will remove the library on disposal. You can pass com.intellij.testFramework.UsefulTestCase#getTestRootDisposable there for that purpose. 

Comment actions Permalink

This seems completely undocumented - at least when 2017.3 is officially released could this please be added to the docs? Lack of documentation is one of the worst aspects of IntelliJ plugin development, so it would be greatly appreciated.

1
Comment actions Permalink

Thanks a lot Alexey. I thought it was something that I am doing in the test was not completely correct.

 

I have to say that I agree with Kyle for this. I am an EAP user and I read all release notes and blog posts for the new versions. However, I haven't seen anything about this. I have already mentioned in the gitter group, I would say it here as well, I think that you need to mention this in one of the blog posts for the next EAP version. I know that it is not interesting and relevant for "normal users', but for plugin development, I think it is quite important and it seems almost everyone has missed it.

 

Thanks again for the help and pointers.

0
Comment actions Permalink

Hey Alexey,

 

I finally got the chance to implement the fix for my tests. However, I couldn't find a removeLibrary() method in PsiTestUtll. I tried using com.intellij.testFramework.PsiTestUtil#addLibrary(com.intellij.openapi.Disposable, com.intellij.openapi.module.Module, java.lang.String, java.lang.String, java.lang.String...), but I am getting the same errors.

I changed the referenced line to:

PsiTestUtil.addLibrary( getTestRootDisposable(), getModule(), "Mapstruct", mapstructLibPath, "mapstruct.jar" );

 

I would appreciate if someone can show me how it can be done just with a remove library in the tearDown(), or with the Disposable. If you point me to some code in the open source version of IntelliJ I might figure it out. Best would be if someone can show me how I can do it for my plugin (it does not have an overly complex test setup).

 

Thanks,

Filip

0
Comment actions Permalink

maybe the library got removed too late. Could you show the code? Or at least tell the class your tests extend from? 

Or call removeLibrary() method in your teardown(), which can be defined like this:

void removeLibrary(Library library) {

WriteCommandAction.runWriteCommandAction(null, ()-> {
  LibraryTable table = ProjectLibraryTable.getInstance(getProject());
  LibraryTable.ModifiableModel model = table.getModifiableModel();
  model.removeLibrary(library);
  model.commit();
 

});

}

0
Comment actions Permalink

Hey Alexey,

 

What I did is change line https://github.com/mapstruct/mapstruct-idea/blob/master/src/test/java/org/mapstruct/intellij/MapstructBaseCompletionTestCase.java#L57 to have 

PsiTestUtil.addLibrary( getTestRootDisposable(), getModule(), "Mapstruct", mapstructLibPath, "mapstruct.jar" );

 nothing else. I am extending from the LightFixtureCompletionTestCase.

Adding your method and doing:

 

private Ref<Library> ref;
@Override
protected void setUp() throws Exception {
super.setUp();
final String mapstructLibPath = PathUtil.toSystemIndependentName( new File( BUILD_LIBS_DIRECTORY )
.getAbsolutePath() );
VfsRootAccess.allowRootAccess( mapstructLibPath );
Module module = getModule();
ref = new Ref<>();
ModuleRootModificationUtil.updateModel( module, model -> {
ref.set( PsiTestUtil.addLibrary( module, model, "Mapstruct", mapstructLibPath, "mapstruct.jar" ) );
} );
}

@Override
public void tearDown() throws Exception {
removeLibrary( ref.get() );
super.tearDown();

}

void removeLibrary(Library library) {
WriteCommandAction.runWriteCommandAction(null, ()-> {
LibraryTable table = ProjectLibraryTable.getInstance(getProject());
LibraryTable.ModifiableModel model = table.getModifiableModel();
model.removeLibrary(library);
model.commit();

});
}

 

Actually works. However, it looks really complex. Is this the recommended way of disposing? To me it looks like one should know the internals of IntelliJ in order to be able to write this (and not all plugins need that much internal knowledge).

 

Cheers,

Filip

0
Comment actions Permalink

Hi Alexey,

 

I managed to do something. However, it seems like I can do my fix only for 2017.3. How can I make my tests work for older versions as well. I have this PR https://github.com/mapstruct/mapstruct-idea/pull/19, that shows that the proposed changes work for 2017.3 (but fail for the other versions as the method signature is different).

0
Comment actions Permalink

By the way, I also update the PR with something that I thing should work for older versions. However, I am getting some "Too many leaked projects" assertion failures.

0
Comment actions Permalink

Edit: Comment was irrelevant. Build fails with same message sporadically

0
Comment actions Permalink

Unfortunately it's the LightFixtureCompletionTestCase which brings more complexity than usual because it's never actually closes its project. To simplify things you can, instead of 

PsiTestUtil.addLibrary( getTestRootDisposable(),...)

use

PsiTestUtil.addLibrary( myFixture.getProjectDisposable(), ...)

which does dispose things earlier.  

0
Comment actions Permalink

Thanks Alexey,

 

I'll try to do that. However, that is only possible for 2017.3. How can I do the disposal in pre 2017.3 tests? I want my plugin to be compatible with older versions, and I don't want to force 2017.3 because the tests there are failing. 

 

Cheers,

Filip

0
Comment actions Permalink

I've found that using 

PsiTestUtil.addLibrary( myFixture.getProjectDisposable(), ...)

Stops the java code style manager in my tests from being able to shorten the class references for annotations that my plugin is adding to the source code meaning my tests will all fail as they are expecting the shortened class references.

Is there another approach?

Raised this as issue:

https://intellij-support.jetbrains.com/hc/en-us/community/posts/360005543680-Correct-way-to-setup-and-teardown-libraries-for-integration-tests

0

Please sign in to leave a comment.