RFR: 8319938: com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array for LAF: javax.swing.plaf.metal.MetalLookAndFeel" [v3]

[Alexey Ivanov]

On Wed, 22 Nov 2023 13:36:31 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Updated.
>
>> And whilst import java.io sorts after java.awt, it is long standing convention that the "core"
>> packages (easily distinguished these days as those in the java.base module) are listed before the desktop / AWT / Swing ones. This is true of product source as well as tests.
> 
> @prrace This is the first time I hear this. In all the classes I've seen in AWT and Swing, the imports are sorted alphabetically with `java.io` after `java.awt`.
> 
> I just looked at [`JComponent`](https://github.com/openjdk/jdk/blob/5e818318eac8cda7d42b599dc7d7d44e5c299a9f/src/java.desktop/share/classes/javax/swing/JComponent.java#L54-L61), [`Component`](https://github.com/openjdk/jdk/blob/5e818318eac8cda7d42b599dc7d7d44e5c299a9f/src/java.desktop/share/classes/java/awt/Component.java#L61-L65), [Font](https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/java/awt/Font.java#L32-L38), and
> 
> https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/sun/font/FontManager.java#L28-L30
> 
> https://github.com/openjdk/jdk/blob/6ce0ebb858d3112f136e12d3ad595f805f6871a0/src/java.desktop/share/classes/sun/font/Font2D.java#L28-L36
> 
> [Sun/Oracle Code Conventions for Java Programming Language](https://www.oracle.com/java/technologies/javase/codeconventions-fileorganization.html#277) say nothing about the order of imports. (The HTML version has absolutely broken formatting now.) [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html#s3.3-import-statements) states, <q cite="https://google.github.io/styleguide/javaguide.html#s3.3-import-statements">…the imported names appear in ASCII sort order.</q>
> 
> Andreas Lundblad's draft for [Java Style Guidelines](https://cr.openjdk.org/~alundblad/styleguide/index-v6.html#toc-import-statements), which is more or less followed by the OpenJDK project, defines the following order:
> 
>> Import statements should be sorted…
>>
>> *  …primarily by non-static / static with non-static imports first.
>> * …secondarily by package origin according to the following order
>>   * `java` packages
>>   * `javax` packages
>>   * external packages (e.g. `org.xml`)
>>   * internal packages (e.g. `com.sun`)
>> * …tertiary by package and class identifier in lexicographical order
> 
> [Andreas](https://github.com/aioobe) opened [PR 14](https://github.com/openjdk/guide/pull/14) to incorporate it into OpenJDK Developers' Guide, but there are objections to making the Java Code Conventions p...

> I don't know why it was necessary to move all around all the above lines.

@prrace I nearly always ask to move the jtreg tags to the class declaration, especially for new tests. When you open the test file in IDE, the copyright block above imports is collapsed, so is the jtreg tags if they're placed above the imports.

If the jtreg tags are placed in a comment that precedes the class declaration, after the imports, they're not collapsed — you can see them right away without scrolling or clicking. I consider the jtreg tags quite relevant to see them easily.

Most older tests place the jtreg tags above the imports; it could've been the requirement or limitation of jtreg at that time. It's not a limitation any more.

As for moving the jtreg tags in existing tests when the test is modified, I also do it occasionally for the reasons outlined above: the jtreg tags describe the test and belong to the class declaration, akin a javadoc comment. (Yet I am against using the javadoc syntax, starting with two asterisks, for the jtreg tasks because it is not a javadoc and because it removes the IDE warnings for the unknown javadoc tasks.)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1402082665