search

eclipse / xtext-xtend

xtext-xtend
Eclipse Public License 2.0
103 stars 53 forks source link

Investigate hang/timeout in JavaRefactoringIntegrationTest with latest #1310

Closed cdietrich closed 2 years ago

cdietrich commented 2 years ago

03:21:49 Running org.eclipse.xtend.ide.tests.refactoring.JavaRefactoringIntegrationTest 09:04:28 Cancelling nested steps due to timeout 09:04:28 Sending interrupt signal to process

https://ci.eclipse.org/xtext/job/xtext-xtend/job/cd_testTycho30/55/console https://ci.eclipse.org/xtext/job/xtext-xtend/job/master/2146/console

cdietrich commented 2 years ago

somehow the logging for timeouts does not log any stacktraces :(

cdietrich commented 2 years ago

Bildschirmfoto vom 2022-03-28 20-41-51 @szarnekow do you have any platform changes in mind that could cause this?

cdietrich commented 2 years ago

org.eclipse.xtend.ide.tests.refactoring.JavaRefactoringIntegrationTest.testRenameJavaMethod()

cdietrich commented 2 years ago

possible candidates https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/70037

cdietrich commented 2 years ago 
RefactoringStatusEntry.<init>(int, String, RefactoringStatusContext) line: 87   
RefactoringStatus.addFatalError(String, RefactoringStatusContext) line: 551 
RenameVirtualMethodProcessor(RenameMethodProcessor).checkNewElementName(String) line: 231   
RenameVirtualMethodProcessor(RenameMethodProcessor).doCheckFinalConditions(IProgressMonitor, CheckConditionsContext) line: 346  
RenameVirtualMethodProcessor.doCheckFinalConditions(IProgressMonitor, CheckConditionsContext) line: 128 
RenameVirtualMethodProcessor(JavaRenameProcessor).checkFinalConditions(IProgressMonitor, CheckConditionsContext) line: 51   
RenameRefactoring(ProcessorBasedRefactoring).checkFinalConditions(IProgressMonitor) line: 226   
RenameRefactoring(Refactoring).checkAllConditions(IProgressMonitor) line: 165   
RefactoringExecutionHelper$Operation.run(IProgressMonitor) line: 84 
BatchOperation.executeOperation() line: 41  
BatchOperation(JavaModelOperation).run(IProgressMonitor) line: 740  
Workspace.run(ICoreRunnable, ISchedulingRule, int, IProgressMonitor) line: 2315 
Workspace.run(IWorkspaceRunnable, ISchedulingRule, int, IProgressMonitor) line: 2340    
JavaCore.run(IWorkspaceRunnable, ISchedulingRule, IProgressMonitor) line: 5929  
WorkbenchRunnableAdapter.run(IProgressMonitor) line: 108    
ModalContext$ModalContextThread.run() line: 122
cdietrich commented 2 years ago

test does

createFile("JavaClass.java", "public class JavaClass { public void foo() {} }");
IMethod foo = findJavaType("JavaClass").getMethod("foo", new String[0]);
renameJavaElement(foo, "baz");

with

    protected void renameJavaElement(IMethod javaElement, String newName) throws CoreException, InterruptedException,
            InvocationTargetException {
        syncUtil.totalSync(false);
        RenameSupport renameSupport = RenameSupport.create(javaElement, newName, RenameSupport.UPDATE_REFERENCES);
        renameSupport.perform(workbench.getActiveWorkbenchWindow().getShell(), workbench.getActiveWorkbenchWindow());
        syncUtil.totalSync(false);
        assertTrue(compositeRefactoringProcessorAccess.isDisposed());
    }
cdietrich commented 2 years ago

RenameMethodProcessor.initialize seems to set the method name to old method name

    protected void initialize(IMethod method) {
        assignMethod(method);
        if (!fInitialized) {
            if (method != null)
                setNewElementName(method.getElementName());
            fUpdateReferences= true;
            initializeWorkingCopyOwner();
        }
    }
cdietrich commented 2 years ago

the initialize call seems to override the newelementname again Bildschirmfoto vom 2022-03-28 21-12-31 Bildschirmfoto vom 2022-03-28 21-13-05

cdietrich commented 2 years ago

cc @jjohnstn

jjohnstn commented 2 years ago

Looking into it.

jjohnstn commented 2 years ago

I believe I have found the issue. I was able to reproduce the problem. I have created a follow-on patch:

https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/192294

Please recheck when the patch has been merged into a new build

cdietrich commented 2 years ago

with upstream fix, test is green again thx @jjohnstn