[PATCH] Circularize selected ways

[qeef]

Dear JOSM developers,

I would like to contribute the code (in the patch set that follows) to the
JOSM. However, I am unable to run the test -- ant test command returns:

BUILD FAILED
/home/jiri/josm/build.xml:507: The following error occurred while executing this line:
/home/jiri/josm/build.xml:448: Problem: failed to create task or type junitlauncher
Cause: the class org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.JUnitLauncherTask was not found.

This looks like one of Ant's optional components.

Action: Check that the appropriate optional JAR exists in

-/usr/share/ant/lib
-/home/jiri/.ant/lib
-a directory added on the command line with the -lib argument

ant-optional is installed, but I could not find ant-optinal.jar:

$ dpkg -l | grep ant-optional
ii ant-optional 1.10.9-4 all Java based build tool like make - optional libraries

$ find / -name *ant-opt* 2>/dev/null
/var/cache/apt/archives/ant-optional_1.10.9-4_all.deb
/var/lib/dpkg/info/ant-optional.md5sums
/var/lib/dpkg/info/ant-optional.list
/usr/share/doc/ant-optional

tested with:

# java -version
openjdk version "1.8.0_222"
OpenJDK Runtime Environment (build 1.8.0_222-8u222-b10-1~deb9u1-b10)
OpenJDK 64-Bit Server VM (build 25.222-b10, mixed mode)

and with:

# java -version
openjdk version "11.0.16" 2022-07-19
OpenJDK Runtime Environment (build 11.0.16+8-post-Debian-1deb11u1)
OpenJDK 64-Bit Server VM (build 11.0.16+8-post-Debian-1deb11u1, mixed mode, sharing)

Have anyone encountered similar issues? Could anyone review the patch without
me running the tests, please?

[taylor.smock]

Hi qeef:

org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.JUnitLauncherTask is not available in the standard repos, IIRC. I think they just whitelist the specific jar files that they want for ant, ant-optional, etc. And the packagers haven't whitelisted the JUnitLauncherTask jar.

Anyway, you can download the full ant package at ​https://ant.apache.org/ .

If you do run tests, please make certain you aren't running integration tests -- the imagery integration test will take hours. Without them, it will be ~20 minutes. Which is long for unit tests.

I usually use something like

$ ant test-html -Dtest.headless=true -Ddefault-junit-IT-excludes='**/*IT*'

Notes on a quick readthrough of the patch:

• In 0001, please add documentation on the new public method. At minimum,

  /**
   * Generic description of moving nodes
   * @param nodes (you can probably copy/paste the description from the method you are coming from)
   * @param fixNodes
   * @param center
   * @param radius
   * @return A command for moving nodes (please put something more descriptive here)
   * @since xxx
   */
  

• In 0002, please change skip_this_way to camel case (e.g. skipThisWay). This is just a style thing.

I would like to see some tests -- since it doesn't look like you are needing to ask the user something, it should be fairly trivial. You'd be surprised how easy it is to break something accidentally.

EDIT: Debian bug: ​https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=967933 -- fixed 2022-07-11, so probably not yet in Ubuntu/Debian stable.

[qeef]

Dear taylor.smock, thank you, I can run ant test now.

You'd be surprised how easy it is to break something accidentally.

Unfortunately no, I would not.

I am sharing the second round of the patches, including the simple tests. I am fine with extending the tests if needed. Thanks.

[taylor.smock]

A couple notes, which I'm fixing locally:

• Don't do assertTrue(false). Instead, use fail("message").
• You don't always have to specify the generic argument (example: new HashSet<Node>() could have been new HashSet<>()). This is due to the method being called defining that argument as Set<Node> fixNodes. But, since it was a new empty set, Collections.emptySet() should have been used.

[taylor.smock]

In 18615/josm:

Fix #22504: Circularize multiple selected ways (patch by qeef, modified)

[skyper]

Documentation at Help/Action/AlignInCircle needs to be extended.