commit 27ef6b4e60e9885a3e1fc5c85b0bd41b82971466
author Tom Anderson <thomasanderson@chromium.org> Mon Aug 13 12:18:19 2018 -0700
committer Commit Bot <commit-bot@chromium.org> Mon Aug 13 20:48:14 2018 +0000
tree f26d90aa899b76fda3fed0bcc605351db2e01790
parent f8f395ac112b2666624f311e18079a05e4fd4a2b

Fix gn_unittests

Scope::KeyValueMap used to be a std::unordered_map, which was confusing the
newly added output conversion unittests that uses string comparison to check if
two values are the same.  The fact that the outputs were unordered means that,
depending on the version of the C++ library used, a value might convert to
"a=0\nb=1" or "b=1\na=0".

This CL makes Scope::KeyValueMap an ordered map instead.

R=brettw@google.com
CC=juliehockett@google.com

Change-Id: Ia3dd1b6a01759f94d1edc96c0bd2a115f743c8c7
Reviewed-on: https://gn-review.googlesource.com/2663
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>

tools/gn/output_conversion_unittest.cc

diff --git a/tools/gn/output_conversion_unittest.cc b/tools/gn/output_conversion_unittest.cc
index 8115796..2e4146d 100644
--- a/tools/gn/output_conversion_unittest.cc
+++ b/tools/gn/output_conversion_unittest.cc
@@ -107,7 +107,7 @@
   ConvertValueToOutput(settings(), Value(nullptr, std::move(new_scope)),
                        Value(nullptr, "string"), result, &err);
   EXPECT_FALSE(err.has_error());
-  EXPECT_EQ(result.str(), "\"{\n  v = \"hello\"\n  _private = \"hello\"\n}\"");
+  EXPECT_EQ(result.str(), "\"{\n  _private = \"hello\"\n  v = \"hello\"\n}\"");
 }
 
 TEST_F(OutputConversionTest, ValueString) {
@@ -171,7 +171,7 @@
   ConvertValueToOutput(settings(), Value(nullptr, std::move(new_scope)),
                        Value(nullptr, "value"), result, &err);
   EXPECT_FALSE(err.has_error());
-  EXPECT_EQ(result.str(), "{\n  v = \"hello\"\n  _private = \"hello\"\n}");
+  EXPECT_EQ(result.str(), "{\n  _private = \"hello\"\n  v = \"hello\"\n}");
 }
 
 TEST_F(OutputConversionTest, JSON) {

tools/gn/scope.h

diff --git a/tools/gn/scope.h b/tools/gn/scope.h
index 10219c0..5e9745a 100644
--- a/tools/gn/scope.h
+++ b/tools/gn/scope.h
@@ -38,8 +38,7 @@
 // variables. So you should use a non-const containing scope whenever possible.
 class Scope {
  public:
-  typedef std::unordered_map<base::StringPiece, Value, base::StringPieceHash>
-      KeyValueMap;
+  typedef std::map<base::StringPiece, Value> KeyValueMap;
   // Holds an owning list of Items.
   typedef std::vector<std::unique_ptr<Item>> ItemVector;
 
@@ -348,7 +347,7 @@
   // scope, or a const containing scope. The reason is that when we're doing
   // a new target, we want to refer to the base_config scope which will be read
   // by multiple threads at the same time, so we REALLY want it to be const.
-  // When you jsut do a nested {}, however, we sometimes want to be able to
+  // When you just do a nested {}, however, we sometimes want to be able to
   // change things (especially marking unused vars).
   const Scope* const_containing_;
   Scope* mutable_containing_;