Minor cleanups in Skylark

--
MOS_MIGRATED_REVID=87535290

5 files changed, 19 insertions, 17 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
index 3d463f3018..1cc8debb7e 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -107,7 +108,7 @@ public class BuildFileASTTest {
                                                + "bar()\n"
                                                + "something = baz()\n"
                                                + "bar()");
-    assertEquals(4, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(4);
   }
 
   @Test
@@ -205,7 +206,7 @@ public class BuildFileASTTest {
                                                             locator, false);
 
     assertFalse(buildFileAST.containsErrors());
-    assertEquals(5, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(5);
   }
 
   @Test
@@ -220,7 +221,7 @@ public class BuildFileASTTest {
                                                             locator, false);
 
     assertFalse(buildFileAST.containsErrors());
-    assertEquals(3, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(3);
 
     Environment env = new Environment();
     Reporter reporter = new Reporter();
@@ -244,7 +245,7 @@ public class BuildFileASTTest {
 
     BuildFileAST buildFileAST = parseBuildFile(fileA);
     assertFalse(buildFileAST.containsErrors());
-    assertEquals(8, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(8);
 
     Environment env = new Environment();
     Reporter reporter = new Reporter();
@@ -257,7 +258,7 @@ public class BuildFileASTTest {
   public void testFailInclude() throws Exception {
     events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include(\"//nonexistent\")");
-    assertEquals(1, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(1);
     events.assertContainsEvent("Include of '//nonexistent' failed");
   }
 
@@ -278,7 +279,7 @@ public class BuildFileASTTest {
         "include(\"//nonexistent:foo\")\n");
     BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
                                                             emptyLocator, false);
-    assertEquals(1, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(1);
     events.assertContainsEvent("Package 'nonexistent' not found");
   }
 
@@ -286,7 +287,7 @@ public class BuildFileASTTest {
   public void testInvalidInclude() throws Exception {
     events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include(2)");
-    assertEquals(0, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).isEmpty();
     events.assertContainsEvent("syntax error at '2'");
   }
 
@@ -319,6 +320,6 @@ public class BuildFileASTTest {
   public void testNonExistentIncludeReported() throws Exception {
     events.setFailFast(false);
     BuildFileAST buildFileAST = parseBuildFile("include('//foo:bar')");
-    assertEquals(1, buildFileAST.getStatements().size());
+    assertThat(buildFileAST.getStatements()).hasSize(1);
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
index 9da4ed2f81..0753b0aa92 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -137,7 +138,7 @@ public class EvalUtilsTest {
 
     // Note: formatString doesn't perform scalar x -> (x) conversion;
     // The %-operator is responsible for that.
-    assertEquals("", EvalUtils.formatString("", makeTuple()));
+    assertThat(EvalUtils.formatString("", makeTuple())).isEmpty();
     assertEquals("foo", EvalUtils.formatString("%s", makeTuple("foo")));
     assertEquals("3.14159", EvalUtils.formatString("%s", makeTuple(3.14159)));
     checkFormatPositionalFails("%s", makeTuple(1, 2, 3),
@@ -205,8 +206,8 @@ public class EvalUtilsTest {
     String prettyWithout = EvalUtils.prettyPrintValue(withoutStripPrefix);
     String prettyWith = EvalUtils.prettyPrintValue(withStripPrefix);
 
-    assertTrue(prettyWithout.contains("strip_prefix = \".\""));
-    assertTrue(prettyWith.contains("strip_prefix = \"orange\""));
+    assertThat(prettyWithout).contains("strip_prefix = \".\"");
+    assertThat(prettyWith).contains("strip_prefix = \"orange\"");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index c31b17b161..932ea3b29a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 
@@ -253,8 +254,7 @@ public class EvaluationTest extends AbstractEvaluationTestCase {
                            final Map<String, Object> kwargs,
                            FuncallExpression ast,
                            Environment env) {
-          ArrayList<String> keys = new ArrayList<>(kwargs.keySet());
-          Collections.sort(keys);
+          List<String> keys = Ordering.natural().sortedCopy(new ArrayList<String>(kwargs.keySet()));
           if ((Integer) args.get(0) == 0) {
             return keys;
           } else {
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index a4fa5d1f9a..9a407d2f2a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -292,7 +292,7 @@ public class ParserTest extends AbstractParserTestCase {
     String expr = "f(1 % 2)";
     FuncallExpression call = (FuncallExpression) parseExpr(expr);
     Argument.Passed arg = call.getArguments().get(0);
-    assertTrue(arg.getLocation().getEndOffset() < call.getLocation().getEndOffset());
+    assertThat(arg.getLocation().getEndOffset()).isLessThan(call.getLocation().getEndOffset());
   }
 
   @Test
@@ -300,7 +300,7 @@ public class ParserTest extends AbstractParserTestCase {
     String expr = "f(1 + 2)";
     FuncallExpression call = (FuncallExpression) parseExpr(expr);
     Argument.Passed arg = call.getArguments().get(0);
-    assertTrue(arg.getLocation().getEndOffset() < call.getLocation().getEndOffset());
+    assertThat(arg.getLocation().getEndOffset()).isLessThan(call.getLocation().getEndOffset());
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index e3ee1e1630..aea87b7a2b 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -13,8 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
@@ -79,7 +79,7 @@ public class SkylarkListTest extends AbstractEvaluationTestCase {
   @Test
   public void testLazyListConcat() throws Exception {
     exec("v = [1, 2] + lazy");
-    assertTrue(env.lookup("v") instanceof SkylarkList);
+    assertThat(env.lookup("v")).isInstanceOf(SkylarkList.class);
   }
 
   @Test