Changes to allow for tab-completion file creation in the GATK. #3424

jonn-smith · 2017-08-09T21:42:53Z

All that was required was pointing to the latest barclay release and
creating a new build target in build.gradle.

cmnbroad

I think it would be worth adding an integration smoke test for tab completion similar to the docgen one. It only runs on a handful of packages, and doesn't attempt to validate the output, but it ensures the process runs successfully. Also it would catch things like the missing template issue we hit yesterday.

cmnbroad · 2017-08-10T13:04:16Z

build.gradle

@@ -495,6 +495,56 @@ task gatkDoc(type: Javadoc, dependsOn: classes) {
    options.addStringOption("verbose")
 }

+// Generate GATK Bash Tab Completion File
+task gatkTabComplete(type: Javadoc, dependsOn: classes) {
+    final File tabCompletionDir = new File("build/docs/tabCompletion")


We should extract a docRoot variable for the value "build/docs" since its repeated in a few places.

Sounds good. I've added in a test. I'll commit the update now.

codecov-io · 2017-08-14T15:17:12Z

Codecov Report

Merging #3424 into master will decrease coverage by 0.224%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##             master     #3424       +/-   ##
==============================================
- Coverage     80.37%   80.146%   -0.224%     
- Complexity    17670     17799      +129     
==============================================
  Files          1179      1184        +5     
  Lines         63876     64565      +689     
  Branches       9930     10055      +125     
==============================================
+ Hits          51337     51746      +409     
- Misses         8584      8842      +258     
- Partials       3955      3977       +22

Impacted Files	Coverage Δ	Complexity Δ
...adinstitute/hellbender/CommandLineProgramTest.java	`100% <100%> (ø)`	`6 <2> (+2)`	⬆️
...nstitute/hellbender/utils/help/GATKHelpDoclet.java	`0% <0%> (-100%)`	`0% <0%> (-6%)`
...titute/hellbender/utils/help/GATKGSONWorkUnit.java	`0% <0%> (-100%)`	`0% <0%> (-2%)`
...lbender/utils/help/GATKHelpDocWorkUnitHandler.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...institute/hellbender/utils/help/HelpConstants.java	`3.704% <0%> (-92.593%)`	`1% <0%> (-7%)`
...am/RevertOriginalBaseQualitiesAndAddMateCigar.java	`0% <0%> (-11.111%)`	`0% <0%> (ø)`
...titute/hellbender/tools/spark/pathseq/PSUtils.java	`73.684% <0%> (-4.094%)`	`21% <0%> (+9%)`
...titute/hellbender/tools/picard/sam/ReorderSam.java	`0% <0%> (-3.488%)`	`0% <0%> (-1%)`
...itute/hellbender/tools/picard/vcf/LiftOverVcf.java	`0% <0%> (-1.149%)`	`0% <0%> (-1%)`
...ine/GATKPlugin/GATKReadFilterPluginDescriptor.java	`86.4% <0%> (-0.8%)`	`50% <0%> (-1%)`
... and 19 more

droazen · 2017-08-18T21:21:58Z

src/test/java/org/broadinstitute/hellbender/utils/test/TabCompletionSmokeTest.java

+        docArgList.addAll(tabCompletionTestPackages);
+
+        // This is  smoke test; we just want to make sure it doesn't blow up
+        int success = com.sun.tools.javadoc.Main.execute(docArgList.toArray(new String[]{}));


This com.sun class reference in the GATK test suite might be an issue, since these classes are not present in every JVM out there.

Maybe we could run javadoc as a process and get the return value to avoid classloading the com.sun class?

Sure. Fixing in new update. I'll fix HelpSmokeTest too.

droazen

One comment on the com.sun reference in our test suite, otherwise looks good. Merge after resolving the com.sun issue.

All that was required was pointing to the latest barclay release and creating a new build target in build.gradle.

…letion doclets.

cmnbroad · 2017-08-24T18:43:30Z

src/test/java/org/broadinstitute/hellbender/utils/test/TabCompletionSmokeTest.java

+//        // This is  smoke test; we just want to make sure it doesn't blow up
+//        int success = com.sun.tools.javadoc.Main.execute(docArgList.toArray(new String[]{}));
+//        Assert.assertEquals(success, 0, "Failure processing gatkTabComplete via javadoc");
+//    }


This can be deleted now.

droazen

Final review complete -- merge after addressing comments.

droazen · 2017-08-24T19:40:58Z

src/test/java/org/broadinstitute/hellbender/utils/test/HelpSmokeTest.java

@@ -2,6 +2,7 @@

 import org.broadinstitute.hellbender.CommandLineProgramTest;
 import org.broadinstitute.hellbender.utils.help.GATKHelpDoclet;
+import org.broadinstitute.hellbender.utils.runtime.ProcessController;
 import org.testng.Assert;
 import org.testng.annotations.Test;



Maybe this class should be called something like DocumentationGenerationIntegrationTest instead of HelpSmokeTest, which is a little puzzling/inconsistent with our test class naming conventions.

Sounds good.

droazen · 2017-08-24T19:41:58Z

src/test/java/org/broadinstitute/hellbender/utils/test/TabCompletionSmokeTest.java

+/**
+ * Smoke test to run tab completion generation on a subset of classes to make sure it doesn't regress.
+ */
+public class TabCompletionSmokeTest extends CommandLineProgramTest {


Use standard GATK test class naming conventions: TabCompletionIntegrationTest

droazen · 2017-08-24T19:43:11Z

src/test/java/org/broadinstitute/hellbender/utils/test/TabCompletionSmokeTest.java

+    static {
+        final ClassFinder classFinder = new ClassFinder();
+        classFinder.find("org.broadinstitute.hellbender", CommandLineProgram.class);
+        tabCompletionTestPackages = classFinder.getClasses().stream().map(Class::getName).collect(Collectors.toList());


Make this an immutable list

droazen · 2017-08-24T19:43:35Z

src/test/java/org/broadinstitute/hellbender/utils/test/TabCompletionSmokeTest.java

+        tabCompletionTestPackages = classFinder.getClasses().stream().map(Class::getName).collect(Collectors.toList());
+    }
+
+//    @Test


Remove commented-out code

jonn-smith assigned droazen Aug 9, 2017

jonn-smith requested review from droazen and lbergelson August 9, 2017 21:42

cmnbroad reviewed Aug 10, 2017

View reviewed changes

droazen changed the title ~~Changes to allow for tab-completion file creation in teh GATK.~~ Changes to allow for tab-completion file creation in the GATK. Aug 11, 2017

droazen reviewed Aug 18, 2017

View reviewed changes

droazen suggested changes Aug 18, 2017

View reviewed changes

droazen assigned jonn-smith and unassigned droazen Aug 18, 2017

jonn-smith added 5 commits August 21, 2017 16:52

Changes to allow for tab-completion file creation in teh GATK.

e3df625

All that was required was pointing to the latest barclay release and creating a new build target in build.gradle.

Adding in Integration Test for the Tab Completion build changes.

43353a2

Minor change for now. Need to fix the fix.

9d2fcbf

Moved a method up to a parent class. Started better test for tab-comp…

fef1cdc

…letion doclets.

Fixed comments from code review.

43c29eb

jonn-smith force-pushed the jts_tab_completion_1454 branch from 683c94f to 43c29eb Compare August 21, 2017 20:52

cmnbroad reviewed Aug 24, 2017

View reviewed changes

droazen suggested changes Aug 24, 2017

View reviewed changes

droazen removed the request for review from lbergelson August 24, 2017 19:45

Addressed latest round of comments.

11c2e31

jonn-smith merged commit 3d6ba41 into master Aug 25, 2017

jonn-smith deleted the jts_tab_completion_1454 branch August 25, 2017 14:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to allow for tab-completion file creation in the GATK. #3424

Changes to allow for tab-completion file creation in the GATK. #3424

jonn-smith commented Aug 9, 2017

cmnbroad left a comment •

edited

Loading

cmnbroad Aug 10, 2017

jonn-smith Aug 14, 2017

codecov-io commented Aug 14, 2017 •

edited

Loading

droazen Aug 18, 2017

droazen Aug 18, 2017

jonn-smith Aug 21, 2017

droazen left a comment

cmnbroad Aug 24, 2017

jonn-smith Aug 24, 2017

droazen left a comment

droazen Aug 24, 2017 •

edited

Loading

jonn-smith Aug 24, 2017

droazen Aug 24, 2017

jonn-smith Aug 24, 2017

droazen Aug 24, 2017

jonn-smith Aug 24, 2017

droazen Aug 24, 2017

jonn-smith Aug 24, 2017

Changes to allow for tab-completion file creation in the GATK. #3424

Changes to allow for tab-completion file creation in the GATK. #3424

Conversation

jonn-smith commented Aug 9, 2017

cmnbroad left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Aug 14, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

droazen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

droazen left a comment

Choose a reason for hiding this comment

droazen Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmnbroad left a comment •

edited

Loading

codecov-io commented Aug 14, 2017 •

edited

Loading

droazen Aug 24, 2017 •

edited

Loading