Skip to content

Commit

Permalink
Reduce Win32 FontRegistries visibility and move tests inside fragment
Browse files Browse the repository at this point in the history
The classes DefaultSWTFontRegistry and ScalingSWTFontRegistry are
currently public only to make them testable from a test bundle. There is
currently no pre-configured way to implement unit tests for classes that
are not directly accessible via public API.

With this change, the SWT Win32 fragments are extended by a
configuration for providing fragment-internal unit tests executed with
plain Maven surefire, which can access the classes under test even with
reduced visibility. Due to current limitation in Tycho, an according
temporary extension to the Maven configuration for the binaries projects
is made. This configuration is applied to the tests for the
DefaultSWTFontRegistry and ScalingSWTFontRegistry, such that their
visibilities can properly be reduced to package visibility.
  • Loading branch information
HeikoKlare committed May 2, 2024
1 parent aab3bfa commit f7aa1c5
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 15 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
bin/
bin_test/
*.log
target/
/.project
Expand Down
6 changes: 6 additions & 0 deletions binaries/.classpath_win32
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,11 @@
<classpathentry kind="src" path="Eclipse SWT Browser/win32"/>
<classpathentry kind="src" path="Eclipse SWT OpenGL/win32"/>
<classpathentry kind="src" path="Eclipse SWT OpenGL/common"/>
<classpathentry kind="src" output="bin_test" path="Eclipse SWT Tests/win32">
<attributes>
<attribute name="test" value="true"/>
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.jdt.junit.JUNIT_CONTAINER/4"/>
<classpathentry kind="output" path="bin"/>
</classpath>
5 changes: 5 additions & 0 deletions binaries/org.eclipse.swt.win32.win32.aarch64/.project
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@
<type>2</type>
<locationURI>SWT_HOST_PLUGIN/Eclipse%20SWT%20WebKit</locationURI>
</link>
<link>
<name>Eclipse SWT Tests</name>
<type>2</type>
<locationURI>SWT_HOST_PLUGIN/Eclipse%20SWT%20Tests</locationURI>
</link>
</linkedResources>
<variableList>
<variable>
Expand Down
3 changes: 3 additions & 0 deletions binaries/org.eclipse.swt.win32.win32.aarch64/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ output.. = bin/
pom.model.property.os=win32
pom.model.property.ws=win32
pom.model.property.arch=aarch64

# To be removed once linked .classpath files are supported by Tycho, see https://github.com/eclipse-tycho/tycho/issues/3803
pom.model.property.localTestsSourceDirectory=../../bundles/org.eclipse.swt/Eclipse SWT Tests/win32
5 changes: 5 additions & 0 deletions binaries/org.eclipse.swt.win32.win32.x86_64/.project
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@
<type>2</type>
<locationURI>SWT_HOST_PLUGIN/Eclipse%20SWT%20WebKit</locationURI>
</link>
<link>
<name>Eclipse SWT Tests</name>
<type>2</type>
<locationURI>SWT_HOST_PLUGIN/Eclipse%20SWT%20Tests</locationURI>
</link>
</linkedResources>
<variableList>
<variable>
Expand Down
3 changes: 3 additions & 0 deletions binaries/org.eclipse.swt.win32.win32.x86_64/build.properties
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ output.. = bin/
pom.model.property.os=win32
pom.model.property.ws=win32
pom.model.property.arch=x86_64

# To be removed once linked .classpath files are supported by Tycho, see https://github.com/eclipse-tycho/tycho/issues/3803
pom.model.property.localTestsSourceDirectory=../../bundles/org.eclipse.swt/Eclipse SWT Tests/win32
24 changes: 24 additions & 0 deletions binaries/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<buildid>${buildId}</buildid>
<maven.compiler.release>17</maven.compiler.release>
<swtMainProject>${project.basedir}/../../bundles/org.eclipse.swt</swtMainProject>
<localTestsSourceDirectory></localTestsSourceDirectory> <!-- to be removed once linked .classpath files are supported by Tycho, see https://github.com/eclipse-tycho/tycho/issues/3803 -->
</properties>

<modules>
Expand All @@ -42,8 +43,31 @@
<module>org.eclipse.swt.win32.win32.x86_64</module>
</modules>

<!-- explicit JUnit dependency to be removed once linked .classpath files are supported by Tycho, see https://github.com/eclipse-tycho/tycho/issues/3803 -->
<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<testSourceDirectory>${localTestsSourceDirectory}</testSourceDirectory> <!-- to be removed once linked .classpath files are supported by Tycho, see https://github.com/eclipse-tycho/tycho/issues/3803 -->
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
<execution>
<id>execute-tests</id>
<goals>
<goal>test</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-apitools-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,19 @@
import org.eclipse.swt.widgets.Display;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class DefaultSWTFontRegistryTests {
private static String TEST_FONT = "Helvetica";
private Display display;
private SWTFontRegistry fontRegistry;

@BeforeClass
public static void assumeIsWindows() {
PlatformSpecificExecution.assumeIsFittingPlatform();
}

@Before
public void setUp() {
this.display = Display.getDefault();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.eclipse.swt.internal;

import static org.junit.Assume.assumeTrue;

import java.net.*;

public final class PlatformSpecificExecution {
private PlatformSpecificExecution() {
}

public static void assumeIsFittingPlatform() {
assumeTrue("test is specific for Windows", isFittingOS());
assumeTrue("architecture of platform does not match", isFittingArchitecture());
}

private static boolean isFittingOS() {
return System.getProperty("os.name").toLowerCase().startsWith("win");
}

private static boolean isFittingArchitecture() {
Class<?> thisClass = PlatformSpecificExecution.class;
String thisClassResourcePath = thisClass.getName().replace('.', '/') + ".class";
URL thisClassURL = thisClass.getClassLoader().getResource(thisClassResourcePath); //$NON-NLS-1$
return thisClassURL.toString().contains(Library.arch());
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
import org.eclipse.swt.widgets.Display;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

public class ScalingSWTFontRegistryTests {
private static String TEST_FONT = "Helvetica";
private SWTFontRegistry fontRegistry;

@BeforeClass
public static void assumeIsWindows() {
PlatformSpecificExecution.assumeIsFittingPlatform();
}

@Before
public void setUp() {
this.fontRegistry = new ScalingSWTFontRegistry(Display.getDefault());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,13 @@
* As this class is only intended to be used internally via {@code SWTFontProvider},
* it should neither be instantiated nor referenced in a client application.
* The behavior can change any time in a future release.
*
* @noreference This class is not intended to be referenced by clients.
* @noinstantiate This class is not intended to be instantiated by clients.
*/
public final class DefaultSWTFontRegistry implements SWTFontRegistry {
final class DefaultSWTFontRegistry implements SWTFontRegistry {
private static FontData KEY_SYSTEM_FONTS = new FontData();
private Map<FontData, Font> fontsMap = new HashMap<>();
private Device device;

public DefaultSWTFontRegistry(Device device) {
DefaultSWTFontRegistry(Device device) {
this.device = device;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@
* As this class is only intended to be used internally via {@code SWTFontProvider},
* it should neither be instantiated nor referenced in a client application.
* The behavior can change any time in a future release.
*
* @noreference This class is not intended to be referenced by clients.
* @noinstantiate This class is not intended to be instantiated by clients.
*/
public final class ScalingSWTFontRegistry implements SWTFontRegistry {
final class ScalingSWTFontRegistry implements SWTFontRegistry {
private class ScaledFontContainer {
// the first (unknown) font to be requested as scaled variant
// usually it is scaled to the primary monitor zoom, but that is not guaranteed
Expand Down Expand Up @@ -72,7 +69,7 @@ private void addScaledFont(int targetZoom, Font scaledFont) {
private Map<FontData, ScaledFontContainer> fontKeyMap = new HashMap<>();
private Device device;

public ScalingSWTFontRegistry(Device device) {
ScalingSWTFontRegistry(Device device) {
this.device = device;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,15 @@
package org.eclipse.swt.tests.win32;

import org.eclipse.swt.graphics.ImageWin32Tests;
import org.eclipse.swt.internal.DefaultSWTFontRegistryTests;
import org.eclipse.swt.internal.ScalingSWTFontRegistryTests;
import org.eclipse.swt.tests.win32.widgets.TestTreeColumn;
import org.eclipse.swt.tests.win32.widgets.Test_org_eclipse_swt_widgets_Display;
import org.junit.runner.JUnitCore;
import org.junit.runner.RunWith;
import org.junit.runners.Suite;


@RunWith(Suite.class)
@Suite.SuiteClasses({
DefaultSWTFontRegistryTests.class,
ImageWin32Tests.class,
ScalingSWTFontRegistryTests.class,
Test_org_eclipse_swt_dnd_DND.class,
Test_org_eclipse_swt_events_KeyEvent.class,
Test_org_eclipse_swt_widgets_Display.class,
Expand Down

0 comments on commit f7aa1c5

Please sign in to comment.