From d6e308f41ea1bc57434911e47ac01a19f76e8895 Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Tue, 30 May 2023 08:54:57 +0100 Subject: [PATCH] JAL-4121 slight refactoring of method name and added tests for headless assumptions/settings --- src/jalview/bin/Jalview.java | 36 +++--------- src/jalview/bin/argparser/BootstrapArgs.java | 50 +++++++++++++++- test/jalview/bin/argparser/ArgParserTest.java | 78 ++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 34 deletions(-) diff --git a/src/jalview/bin/Jalview.java b/src/jalview/bin/Jalview.java index 7535f60..dc97549 100755 --- a/src/jalview/bin/Jalview.java +++ b/src/jalview/bin/Jalview.java @@ -396,7 +396,7 @@ public class Jalview // get bootstrap properties (mainly for the logger level) Properties bootstrapProperties = Cache - .bootstrapProperties(bootstrapArgs.get(Arg.PROPS)); + .bootstrapProperties(bootstrapArgs.getValue(Arg.PROPS)); // report Jalview version Cache.loadBuildProperties( @@ -482,7 +482,7 @@ public class Jalview }); String usrPropsFile = bootstrapArgs.contains(Arg.PROPS) - ? bootstrapArgs.get(Arg.PROPS) + ? bootstrapArgs.getValue(Arg.PROPS) : aparser.getValue("props"); // if usrPropsFile == null, loadProperties will use the Channel // preferences.file @@ -536,7 +536,7 @@ public class Jalview } // new CLI - headlessArg = isHeadless(bootstrapArgs); + headlessArg = bootstrapArgs.isHeadless(); if (headlessArg) { System.setProperty("java.awt.headless", "true"); @@ -553,7 +553,7 @@ public class Jalview // allow https handshakes to download intermediate certs if necessary System.setProperty("com.sun.security.enableAIAcaIssuers", "true"); - String jabawsUrl = bootstrapArgs.get(Arg.JABAWS); + String jabawsUrl = bootstrapArgs.getValue(Arg.JABAWS); if (jabawsUrl == null) jabawsUrl = aparser.getValue("jabaws"); if (jabawsUrl != null) @@ -645,7 +645,7 @@ public class Jalview if (!(headless || headlessArg)) { - Desktop.nosplash = "false".equals(bootstrapArgs.get(Arg.SPLASH)) + Desktop.nosplash = "false".equals(bootstrapArgs.getValue(Arg.SPLASH)) || aparser.contains("nosplash") || Cache.getDefault("SPLASH", "true").equals("false"); desktop = new Desktop(); @@ -774,8 +774,8 @@ public class Jalview if ((!aparser.contains("nonews") && Cache.getProperty("NONEWS") == null - && !"false".equals(bootstrapArgs.get(Arg.NEWS))) - || "true".equals(bootstrapArgs.get(Arg.NEWS))) + && !"false".equals(bootstrapArgs.getValue(Arg.NEWS))) + || "true".equals(bootstrapArgs.getValue(Arg.NEWS))) { desktop.checkForNews(); } @@ -1886,26 +1886,4 @@ public class Jalview System.out.println("[TESTOUTPUT] arg " + (yes ? a.argString() : a.negateArgString()) + " was set"); } - - private static boolean isHeadless(BootstrapArgs bootstrapArgs) - { - if (bootstrapArgs == null) - { - return false; - } - boolean isHeadless = false; - if (bootstrapArgs.contains(Arg.GUI)) - { - isHeadless = !bootstrapArgs.getBoolean(Arg.GUI); - } - else if (bootstrapArgs.contains(Arg.HEADLESS)) - { - isHeadless = bootstrapArgs.getBoolean(Arg.HEADLESS); - } - else if (bootstrapArgs.argsHaveOption(Opt.OUTPUTFILE)) - { - isHeadless = true; - } - return isHeadless; - } } diff --git a/src/jalview/bin/argparser/BootstrapArgs.java b/src/jalview/bin/argparser/BootstrapArgs.java index a6bad24..e1ad1d7 100644 --- a/src/jalview/bin/argparser/BootstrapArgs.java +++ b/src/jalview/bin/argparser/BootstrapArgs.java @@ -25,6 +25,8 @@ public class BootstrapArgs private Set argsOptions = new HashSet<>(); + private Set argsTypes = new HashSet<>(); + public static BootstrapArgs getBootstrapArgs(String[] args) { List argList = new ArrayList<>(Arrays.asList(args)); @@ -122,6 +124,11 @@ public class BootstrapArgs argsOptions.add(opt); } } + Type t = a.getType(); + if (!argsTypes.contains(t)) + { + argsTypes.add(t); + } } if (a == null || !a.hasOption(Opt.BOOTSTRAP)) @@ -275,7 +282,7 @@ public class BootstrapArgs * Retrieves the first value even if MULTI. * A convenience for non-MULTI args. */ - public String get(Arg a) + public String getValue(Arg a) { if (!bootstrapArgMap.containsKey(a)) return null; @@ -287,7 +294,7 @@ public class BootstrapArgs { if (!bootstrapArgMap.containsKey(a)) return d; - return Boolean.parseBoolean(get(a)); + return Boolean.parseBoolean(getValue(a)); } public boolean getBoolean(Arg a) @@ -298,7 +305,7 @@ public class BootstrapArgs } if (bootstrapArgMap.containsKey(a)) { - return Boolean.parseBoolean(get(a)); + return Boolean.parseBoolean(getValue(a)); } else { @@ -310,4 +317,41 @@ public class BootstrapArgs { return argsOptions.contains(opt); } + + public boolean argsHaveType(Type type) + { + return argsTypes.contains(type); + } + + public boolean isHeadless() + { + boolean isHeadless = false; + if (this.argsHaveType(Type.HELP)) + { + // --help, --help-all, ... specified => headless + isHeadless = true; + } + else if (this.contains(Arg.VERSION)) + { + // --version specified => headless + isHeadless = true; + } + else if (this.contains(Arg.GUI)) + { + // --gui specified => forced NOT headless + isHeadless = !this.getBoolean(Arg.GUI); + } + else if (this.contains(Arg.HEADLESS)) + { + // --headless, --noheadless specified => use value + isHeadless = this.getBoolean(Arg.HEADLESS); + } + else if (this.argsHaveOption(Opt.OUTPUTFILE)) + { + // --output file.fa, --image pic.png, --structureimage struct.png => + // assume headless unless above has been already specified + isHeadless = true; + } + return isHeadless; + } } diff --git a/test/jalview/bin/argparser/ArgParserTest.java b/test/jalview/bin/argparser/ArgParserTest.java index ca63e44..461738b 100644 --- a/test/jalview/bin/argparser/ArgParserTest.java +++ b/test/jalview/bin/argparser/ArgParserTest.java @@ -90,7 +90,7 @@ public class ArgParserTest Assert.assertTrue(b.contains(a)); if (a == Arg.PROPS) { - Properties bP = Cache.bootstrapProperties(b.get(Arg.PROPS)); + Properties bP = Cache.bootstrapProperties(b.getValue(Arg.PROPS)); Assert.assertNotNull(bP); Assert.assertTrue(other.equals(bP.get(Cache.BOOTSTRAP_TEST))); Assert.assertFalse(bP.contains("NOT" + Cache.BOOTSTRAP_TEST)); @@ -130,7 +130,7 @@ public class ArgParserTest Assert.assertFalse(b.contains(Arg.APPEND)); if (a == Arg.PROPS) { - Properties bP = Cache.bootstrapProperties(b.get(Arg.PROPS)); + Properties bP = Cache.bootstrapProperties(b.getValue(Arg.PROPS)); Assert.assertTrue("true".equals(bP.get(Cache.BOOTSTRAP_TEST))); } } @@ -340,4 +340,78 @@ public class ArgParserTest // }; } + + @Test(groups = "Functional", dataProvider = "bootstrapArgsData") + public void bootstrapArgsValuesTest(String commandLineArgs, Arg a, + String valS, boolean valB, boolean headlessValue) + { + String[] args = commandLineArgs.split("\\s+"); + BootstrapArgs bsa = BootstrapArgs.getBootstrapArgs(args); + if (a != null) + { + if (valS != null) + { + Assert.assertEquals(bsa.getValue(a), valS, + "BootstrapArg " + a.argString() + + " value does not match expected '" + valS + "'"); + } + else + { + Assert.assertEquals(bsa.getBoolean(a), valB, + "Boolean/Unary value of BootstrapArg " + a.argString() + + "' is not the expected '" + valB + "'"); + } + } + + boolean isHeadless = bsa.isHeadless(); + Assert.assertEquals(isHeadless, headlessValue, + "Assumed headless setting '" + isHeadless + "' is wrong."); + } + + @DataProvider(name = "bootstrapArgsData") + public Object[][] bootstrapArgsData() + { + return new Object[][] { + /* + * cmdline args + * Arg (null if only testing headless) + * String value if there is one (null otherwise) + * boolean value if String value is null + * expected value of isHeadless() + */ + /* + */ + { "--open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.JABAWS, "https://forwardsandbackwards.com/", false, true }, + { "--help-all --open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.HELP, null, true, true }, + { "--help-all --nonews --open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.NEWS, null, false, true }, + { "--help --nonews --open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.NEWS, null, false, true }, + { "--help-opening --nonews --open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.NEWS, null, false, true }, + { "--nonews --open thisway.fa --output thatway.fa --jabaws https://forwardsandbackwards.com/", + Arg.NEWS, null, false, true }, + { "--open thisway.fa --image thatway.png", null, null, false, + true }, + { "--open thisway.fa --output thatway.png", null, null, false, + true }, + { "--open thisway.fa --image thatway.png --noheadless", null, null, + false, false }, + { "--open thisway.fa --output thatway.png --noheadless", null, null, + false, false }, + { "--open thisway.fa --image thatway.png --gui", null, null, false, + false }, + { "--open thisway.fa --output thatway.png --gui", null, null, false, + false }, + // --gui takes precedence + { "--open thisway.fa --image thatway.png --gui --headless", null, + null, false, false }, + { "--open thisway.fa --output thatway.png --gui --headless", null, + null, false, false }, + // + }; + } + } -- 1.7.10.2