Better indentation when adding GCP libraries to pom.xml by lak-proddev · Pull Request #3511 · GoogleCloudPlatform/google-cloud-eclipse

lak-proddev · 2019-06-27T16:21:03Z

Change-Id: I5d4a22b089f1516845e8597cf11c5198772dc4dd

…atform#3381 Change-Id: I5d4a22b089f1516845e8597cf11c5198772dc4dd

elharo

I'm not yet sure if this is the right approach but if it is, it needs a test.

codecov · 2019-07-06T13:26:19Z

Codecov Report

❗ No coverage uploaded for pull request base (master@f8c6895). Click here to learn what that means.
The diff coverage is 94.44%.

@@            Coverage Diff            @@
##             master    #3511   +/-   ##
=========================================
  Coverage          ?   70.10%           
  Complexity        ?     2990           
=========================================
  Files             ?      376           
  Lines             ?    13632           
  Branches          ?     1605           
=========================================
  Hits              ?     9557           
  Misses            ?     3432           
  Partials          ?      643

Impacted Files	Coverage Δ	Complexity Δ
...e/cloud/tools/eclipse/appengine/libraries/Pom.java	`86.90% <94.44%> (ø)`	`60.00 <5.00> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8c6895...741ded9. Read the comment docs.

...ine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/POMFormatTest.java

elharo · 2019-07-06T15:17:14Z

...ine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/PomFormatTest.java

-      }
-    }
+  private static void assertFileContentsEqual(File actual, File expected) throws IOException {
+    assertArrayEquals(Files.readAllBytes(actual.toPath()), Files.readAllBytes(expected.toPath()));


swap arguments since expected customarily comes before actual

elharo · 2019-07-06T15:17:46Z

...ine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/PomFormatTest.java

-    File expected = new File(FileLocator.resolve(fileURL).toURI());
+    URL fileUrl = bundle.getEntry("/testdata/formatRemove.xml");
+    File expected = new File(FileLocator.resolve(fileUrl).toURI());
    assertFileContentsEqual(pomFile.getLocation().toFile(), expected);


swap arguments since expected customarily comes before actual

elharo

Tests are failing:

Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.634 s <<< FAILURE! - in com.google.cloud.tools.eclipse.appengine.libraries.PomFormatTest
testAddDependencies(com.google.cloud.tools.eclipse.appengine.libraries.PomFormatTest) Time elapsed: 0.36 s <<< FAILURE!
java.lang.AssertionError: array lengths differed, expected.length=1168 actual.length=1135
at org.junit.Assert.fail(Assert.java:88)
at org.junit.internal.ComparisonCriteria.assertArraysAreSameLength(ComparisonCriteria.java:76)
at org.junit.internal.ComparisonCriteria.arrayEquals(ComparisonCriteria.java:37)
at org.junit.Assert.internalArrayEquals(Assert.java:532)

lak-proddev

Tests are failing:

Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.634 s <<< FAILURE! - in com.google.cloud.tools.eclipse.appengine.libraries.PomFormatTest
testAddDependencies(com.google.cloud.tools.eclipse.appengine.libraries.PomFormatTest) Time elapsed: 0.36 s <<< FAILURE!
java.lang.AssertionError: array lengths differed, expected.length=1168 actual.length=1135

Not failing locally. will check

lak-proddev · 2019-06-28T03:11:16Z

....eclipse.appengine.libraries/src/com/google/cloud/tools/eclipse/appengine/libraries/Pom.java

  private void writeDocument() throws CoreException, TransformerException {
    Transformer transformer = transformerFactory.newTransformer();
    transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+    transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount","2"); //$NON-NLS-1$ //$NON-NLS-2$


This is the core fix. Other changes are for removing extra spaces and blank line when add or removing libaries.

elharo

Sorry. Just noticed this was still out for review.

elharo · 2019-08-20T18:03:50Z

...ine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/PomFormatTest.java

+      return;
+    }
+
+    final List<String> diff = new ArrayList<>();


Google style avoids final on local variables where not required.

elharo · 2019-08-20T18:04:50Z

....eclipse.appengine.libraries/src/com/google/cloud/tools/eclipse/appengine/libraries/Pom.java

+
+  private void addChild(Node parent, Node newChild) {
+    parent.appendChild(newChild);
+    removeWhiteSpaceBefore(newChild);// for child indentation


nit: space before //

elharo · 2019-08-20T18:04:59Z

....eclipse.appengine.libraries/src/com/google/cloud/tools/eclipse/appengine/libraries/Pom.java

+  }
+
+  private static void removeWhiteSpaceBefore(Node node) {
+    Node prevElement = node.getPreviousSibling();


previousElement

Better indentation when adding GCP libraries to pom.xml GoogleCloudPl…

5d028af

…atform#3381 Change-Id: I5d4a22b089f1516845e8597cf11c5198772dc4dd

googlebot added the cla: yes label Jun 27, 2019

chanseokoh added the kokoro:run label Jun 27, 2019

kokoro-team removed the kokoro:run label Jun 27, 2019

elharo reviewed Jun 27, 2019

View reviewed changes

GoogleCloudPlatform#3381 - test case added

5f56823

elharo reviewed Jul 6, 2019

View reviewed changes

GoogleCloudPlatform#3381 - test case updated

8194366

elharo reviewed Jul 6, 2019

View reviewed changes

GoogleCloudPlatform#3381 - test case updated

45c771d

elharo reviewed Jul 6, 2019

View reviewed changes

lak-proddev commented Jul 7, 2019

View reviewed changes

lak-proddev and others added 2 commits July 7, 2019 09:25

GoogleCloudPlatform#3381 - test case updated

38d59ea

Typo

741ded9

elharo reviewed Aug 20, 2019

View reviewed changes

elharo changed the title ~~Better indentation when adding GCP libraries to pom.xml #3381~~ Better indentation when adding GCP libraries to pom.xml Jun 17, 2021

Conversation

lak-proddev commented Jun 27, 2019 • edited by elharo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elharo Jul 6, 2019

Choose a reason for hiding this comment

Uh oh!

elharo Jul 6, 2019

Choose a reason for hiding this comment

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

lak-proddev left a comment

Choose a reason for hiding this comment

Uh oh!

lak-proddev Jun 28, 2019

Choose a reason for hiding this comment

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

elharo Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

elharo Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

elharo Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lak-proddev commented Jun 27, 2019 •

edited by elharo

Loading

codecov bot commented Jul 6, 2019 •

edited

Loading