Skip to content

Commit

Permalink
Bug 574227 - [test] fixed testSourceGenAfterDirChange
Browse files Browse the repository at this point in the history
The creation of apt directories is not atomic and is done in two
threads in parallel. A race which thread won.

During some tests the builder thread was even so much delayed that the
folders have not been created at all.

Change-Id: I9b4e77ebf48b131ffd6c7559e7361f0e732ad8c5
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182042
Tested-by: JDT Bot <jdt-bot@eclipse.org>
Reviewed-by: Andrey Loskutov <loskutov@gmx.de>
  • Loading branch information
EcljpseB0T authored and iloveeclipse committed Jun 18, 2021
1 parent 3d68a94 commit ccabede
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,15 @@ public class GeneratedSourceFolderManager {
* on when we get notified of relevant changes and on what locks we are
* able to obtain.
*/
private IFolder _generatedSourceFolder = null;
private volatile IFolder _generatedSourceFolder;

private final boolean _isTestCode;


/**
* Reflects whether apt is enabled as soon as it is enabled.
* Only write access this in the thread which receives preferenceChanged:
*/
private volatile boolean _aptEnabled;

/**
* Should be constructed only by AptProject. Other clients should call
Expand All @@ -103,7 +108,8 @@ public GeneratedSourceFolderManager(AptProject aptProject, boolean isTestCode)
// Set _generatedSourceFolder only if APT is enabled, the folder exists,
// and the folder is on the classpath.
// Otherwise leave it null, which will cause us to try to fix things later on.
if (AptConfig.isEnabled(javaProject)) {
_aptEnabled = AptConfig.isEnabled(javaProject);
if (_aptEnabled) {
final IFolder folder = getFolder();
if (folder.exists()) {
if (isOnClasspath(folder)) {
Expand Down Expand Up @@ -200,7 +206,7 @@ public void ensureFolderExists(IFolder srcFolder) {
*/
public void ensureFolderExists(){
// If APT is disabled, do nothing.
if (!AptConfig.isEnabled(_aptProject.getJavaProject())) {
if (!_aptEnabled) {
return;
}

Expand Down Expand Up @@ -259,13 +265,18 @@ public void enabledPreferenceChanged()
{
final boolean enable = AptConfig.isEnabled(_aptProject.getJavaProject());
// Short-circuit if nothing changed.
if (enable == (_generatedSourceFolder != null)) {
if (enable == _aptEnabled) {
if( AptPlugin.DEBUG ) {
AptPlugin.trace("enabledChanged() doing nothing; state is already " + enable); //$NON-NLS-1$
}
// no change in state
return;
}
enabledPreferenceChangedTo(enable);
}

private void enabledPreferenceChangedTo(final boolean enable) {
_aptEnabled = enable;

if ( AptPlugin.DEBUG ) {
AptPlugin.trace("enabledChanged() changing state to " + enable + //$NON-NLS-1$
Expand All @@ -290,8 +301,7 @@ public void enabledPreferenceChanged()
public void folderNamePreferenceChanged()
{
// if APT is disabled, we don't need to do anything
final boolean aptEnabled = AptConfig.isEnabled(_aptProject.getJavaProject());
if (!aptEnabled) {
if (!_aptEnabled) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,11 @@ public static void setEnabled(IJavaProject jproject, boolean enabled) {
enabled ? AptPreferenceConstants.ENABLED : AptPreferenceConstants.DISABLED);
// backward compatibility: also save old setting
setBoolean(jproject, AptPreferenceConstants.APT_ENABLED, enabled);
if (enabled) {
AptProject aptProject = AptPlugin.getAptProject(jproject);
aptProject.getGeneratedSourceFolderManager(true).ensureFolderExists();
aptProject.getGeneratedSourceFolderManager(false).ensureFolderExists();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void testMessagerAPI() throws Exception {
_logListener.clear();
fullBuild( project.getFullPath() );
expectingNoProblems();
assertTrue(_logListener.getList().isEmpty());
assertTrue(_logListener.getList().toString(), _logListener.getList().isEmpty());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ private void assertNoUnexpectedProblems() {

public void testMirrorUtils() throws Exception
{
assertEquals(ProcessorTestStatus.NO_ERRORS, ProcessorTestStatus.getErrors());
assertEquals("Unexpected errors: " + ProcessorTestStatus.getErrors(), ProcessorTestStatus.NO_ERRORS,
ProcessorTestStatus.getErrors());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public void testConfigGenSrcDir() throws Exception {
testFolderName = testSrcFolder.getProjectRelativePath().toOSString();
// test 12: make sure we deleted the source folder when we disable apt
assertEquals(false, srcFolder.exists());
assertEquals(false, testSrcFolder.exists());
assertEquals("testSrcFolder '"+testSrcFolder+"' does still exist",false, testSrcFolder.exists());
// test 13: make sure we didn't overwrite the configure folder name
assertEquals(newName, folderName);
assertEquals(newTestName, testFolderName);
Expand Down

0 comments on commit ccabede

Please sign in to comment.