Merge branch 'json-oj-redux' into 'master'

Gitlab::Json Oj implementation

See merge request gitlab-org/gitlab!36555

Gemfile

@@ -502,3 +502,5 @@ gem 'valid_email', '~> 0.1'
 # JSON
 gem 'json', '~> 2.3.0'
 gem 'json-schema', '~> 2.8.0'
+gem 'oj', '~> 3.10.6'
+gem 'multi_json', '~> 1.14.1'

Gemfile.lock

@@ -687,6 +687,7 @@ GEM
     octokit (4.15.0)
       faraday (>= 0.9)
       sawyer (~> 0.8.0, >= 0.5.3)
+    oj (3.10.6)
     omniauth (1.9.0)
       hashie (>= 3.4.6, < 3.7.0)
       rack (>= 1.6.2, < 3)
@@ -1312,6 +1313,7 @@ DEPENDENCIES
   mimemagic (~> 0.3.2)
   mini_magick
   minitest (~> 5.11.0)
+  multi_json (~> 1.14.1)
   nakayoshi_fork (~> 0.0.4)
   net-ldap
   net-ntp
@@ -1319,6 +1321,7 @@ DEPENDENCIES
   nokogiri (~> 1.10.9)
   oauth2 (~> 1.4)
   octokit (~> 4.15)
+  oj (~> 3.10.6)
   omniauth (~> 1.8)
   omniauth-auth0 (~> 2.0.0)
   omniauth-authentiq (~> 0.3.3)

changelogs/unreleased/swap-to-oj.yml

+---
+title: Add oj gem for faster JSON
+merge_request: 36555
+author:
+type: performance

config/initializers/multi_json.rb

+# frozen_string_literal: true
+
+# Explicitly set the JSON adapter used by MultiJson
+# Currently we want this to default to the existing json gem
+MultiJson.use(:json_gem)

config/initializers/oj.rb

+# frozen_string_literal: true
+
+# Ensure Oj runs in json-gem compatibility mode by default
+Oj.default_options = { mode: :rails }

lib/gitlab/json.rb

 # frozen_string_literal: true
 
+# This is a GitLab-specific JSON interface. You should use this instead
+# of using `JSON` directly. This allows us to swap the adapter and handle
+# legacy issues.
+
 module Gitlab
   module Json
     INVALID_LEGACY_TYPES = [String, TrueClass, FalseClass].freeze
 
     class << self
-      def parse(string, *args, **named_args)
-        legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode))
-        data = adapter.parse(string, *args, **named_args)
+      # Parse a string and convert it to a Ruby object
+      #
+      # @param string [String] the JSON string to convert to Ruby objects
+      # @param opts [Hash] an options hash in the standard JSON gem format
+      # @return [Boolean, String, Array, Hash]
+      # @raise [JSON::ParserError] raised if parsing fails
+      def parse(string, opts = {})
+        # First we should ensure this really is a string, not some other
+        # type which purports to be a string. This handles some legacy
+        # usage of the JSON class.
+        string = string.to_s unless string.is_a?(String)
+
+        legacy_mode = legacy_mode_enabled?(opts.delete(:legacy_mode))
+        data = adapter_load(string, opts)
 
         handle_legacy_mode!(data) if legacy_mode
 
         data
       end
 
-      def parse!(string, *args, **named_args)
-        legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode))
-        data = adapter.parse!(string, *args, **named_args)
+      alias_method :parse!, :parse
+
+      # Restricted method for converting a Ruby object to JSON. If you
+      # need to pass options to this, you should use `.generate` instead,
+      # as the underlying implementation of this varies wildly based on
+      # the adapter in use.
+      #
+      # @param object [Object] the object to convert to JSON
+      # @return [String]
+      def dump(object)
+        adapter_dump(object)
+      end
 
-        handle_legacy_mode!(data) if legacy_mode
+      # Generates JSON for an object. In Oj this takes fewer options than .dump,
+      # in the JSON gem this is the only method which takes an options argument.
+      #
+      # @param object [Hash, Array, Object] must be hash, array, or an object that responds to .to_h or .to_json
+      # @param opts [Hash] an options hash with fewer supported settings than .dump
+      # @return [String]
+      def generate(object, opts = {})
+        adapter_generate(object, opts)
+      end
 
-        data
+      # Generates JSON for an object and makes it look purdy
+      #
+      # The Oj variant in this looks seriously weird but these are the settings
+      # needed to emulate the style generated by the JSON gem.
+      #
+      # NOTE: This currently ignores Oj, because Oj doesn't generate identical
+      #       formatting, issue: https://github.com/ohler55/oj/issues/608
+      #
+      # @param object [Hash, Array, Object] must be hash, array, or an object that responds to .to_h or .to_json
+      # @param opts [Hash] an options hash with fewer supported settings than .dump
+      # @return [String]
+      def pretty_generate(object, opts = {})
+        ::JSON.pretty_generate(object, opts)
       end
 
-      def dump(*args)
-        adapter.dump(*args)
+      private
+
+      # Convert JSON string into Ruby through toggleable adapters.
+      #
+      # Must rescue adapter-specific errors and return `parser_error`, and
+      # must also standardize the options hash to support each adapter as
+      # they all take different options.
+      #
+      # @param string [String] the JSON string to convert to Ruby objects
+      # @param opts [Hash] an options hash in the standard JSON gem format
+      # @return [Boolean, String, Array, Hash]
+      # @raise [JSON::ParserError]
+      def adapter_load(string, *args, **opts)
+        opts = standardize_opts(opts)
+
+        if enable_oj?
+          Oj.load(string, opts)
+        else
+          ::JSON.parse(string, opts)
+        end
+      rescue Oj::ParseError, Encoding::UndefinedConversionError => ex
+        raise parser_error.new(ex)
       end
 
-      def generate(*args)
-        adapter.generate(*args)
+      # Take a Ruby object and convert it to a string. This method varies
+      # based on the underlying JSON interpreter. Oj treats this like JSON
+      # treats `.generate`. JSON.dump takes no options.
+      #
+      # This supports these options to ensure this difference is recorded here,
+      # as it's very surprising. The public interface is more restrictive to
+      # prevent adapter-specific options being passed.
+      #
+      # @overload adapter_dump(object, opts)
+      #   @param object [Object] the object to convert to JSON
+      #   @param opts [Hash] options as named arguments, only supported by Oj
+      #
+      # @overload adapter_dump(object, anIO, limit)
+      #   @param object [Object] the object, will have JSON.generate called on it
+      #   @param anIO [Object] an IO-like object that responds to .write, default nil
+      #   @param limit [Fixnum] the nested array/object limit, default nil
+      #   @raise [ArgumentError] when depth limit exceeded
+      #
+      # @return [String]
+      def adapter_dump(object, *args, **opts)
+        if enable_oj?
+          Oj.dump(object, opts)
+        else
+          ::JSON.dump(object, *args)
+        end
       end
 
-      def pretty_generate(*args)
-        adapter.pretty_generate(*args)
+      # Generates JSON for an object but with fewer options, using toggleable adapters.
+      #
+      # @param object [Hash, Array, Object] must be hash, array, or an object that responds to .to_h or .to_json
+      # @param opts [Hash] an options hash with fewer supported settings than .dump
+      # @return [String]
+      def adapter_generate(object, opts = {})
+        opts = standardize_opts(opts)
+
+        if enable_oj?
+          Oj.generate(object, opts)
+        else
+          ::JSON.generate(object, opts)
+        end
       end
 
-      private
+      # Take a JSON standard options hash and standardize it to work across adapters
+      # An example of this is Oj taking :symbol_keys instead of :symbolize_names
+      #
+      # @param opts [Hash, Nil]
+      # @return [Hash]
+      def standardize_opts(opts)
+        opts ||= {}
+
+        if enable_oj?
+          opts[:mode] = :rails
+          opts[:symbol_keys] = opts[:symbolize_keys] || opts[:symbolize_names]
+        end
 
-      def adapter
-        ::JSON
+        opts
       end
 
+      # The standard parser error we should be returning. Defined in a method
+      # so we can potentially override it later.
+      #
+      # @return [JSON::ParserError]
       def parser_error
         ::JSON::ParserError
       end
 
+      # @param [Nil, Boolean] an extracted :legacy_mode key from the opts hash
+      # @return [Boolean]
       def legacy_mode_enabled?(arg_value)
         arg_value.nil? ? false : arg_value
       end
 
+      # If legacy mode is enabled, we need to raise an error depending on the values
+      # provided in the string. This will be deprecated.
+      #
+      # @param data [Boolean, String, Array, Hash, Object]
+      # @return [Boolean, String, Array, Hash, Object]
+      # @raise [JSON::ParserError]
       def handle_legacy_mode!(data)
+        return data unless feature_table_exists?
         return data unless Feature.enabled?(:json_wrapper_legacy_mode, default_enabled: true)
 
         raise parser_error if INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) }
       end
+
+      # @return [Boolean]
+      def enable_oj?
+        return false unless feature_table_exists?
+
+        Feature.enabled?(:oj_json, default_enabled: true)
+      end
+
+      # There are a variety of database errors possible when checking the feature
+      # flags at the wrong time during boot, e.g. during migrations. We don't care
+      # about these errors, we just need to ensure that we skip feature detection
+      # if they will fail.
+      #
+      # @return [Boolean]
+      def feature_table_exists?
+        Feature::FlipperFeature.table_exists?
+      rescue
+        false
+      end
     end
   end
 end

lib/gitlab/json_logger.rb

@@ -19,7 +19,7 @@ module Gitlab
         data.merge!(message)
       end
 
-      data.to_json + "\n"
+      Gitlab::Json.dump(data) + "\n"
     end
   end
 end

spec/lib/gitlab/ci/parsers/accessibility/pa11y_spec.rb

 # frozen_string_literal: true
 
-require 'fast_spec_helper'
+require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Parsers::Accessibility::Pa11y do
   describe '#parse!' do
@@ -108,7 +108,7 @@ RSpec.describe Gitlab::Ci::Parsers::Accessibility::Pa11y do
       it "sets error_message" do
         expect { subject }.not_to raise_error
 
-        expect(accessibility_report.error_message).to include('Pa11y parsing failed')
+        expect(accessibility_report.error_message).to include('JSON parsing failed')
         expect(accessibility_report.errors_count).to eq(0)
         expect(accessibility_report.passes_count).to eq(0)
         expect(accessibility_report.scans_count).to eq(0)

spec/lib/gitlab/json_spec.rb

@@ -7,189 +7,330 @@ RSpec.describe Gitlab::Json do
     stub_feature_flags(json_wrapper_legacy_mode: true)
   end
 
-  describe ".parse" do
-    context "legacy_mode is disabled by default" do
-      it "parses an object" do
-        expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
-      end
+  shared_examples "json" do
+    describe ".parse" do
+      context "legacy_mode is disabled by default" do
+        it "parses an object" do
+          expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
+        end
 
-      it "parses an array" do
-        expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
-      end
+        it "parses an array" do
+          expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+        end
 
-      it "parses a string" do
-        expect(subject.parse('"foo"', legacy_mode: false)).to eq("foo")
-      end
+        it "parses a string" do
+          expect(subject.parse('"foo"', legacy_mode: false)).to eq("foo")
+        end
 
-      it "parses a true bool" do
-        expect(subject.parse("true", legacy_mode: false)).to be(true)
-      end
+        it "parses a true bool" do
+          expect(subject.parse("true", legacy_mode: false)).to be(true)
+        end
 
-      it "parses a false bool" do
-        expect(subject.parse("false", legacy_mode: false)).to be(false)
+        it "parses a false bool" do
+          expect(subject.parse("false", legacy_mode: false)).to be(false)
+        end
       end
-    end
 
-    context "legacy_mode is enabled" do
-      it "parses an object" do
-        expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
-      end
+      context "legacy_mode is enabled" do
+        it "parses an object" do
+          expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+        end
 
-      it "parses an array" do
-        expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
-      end
+        it "parses an array" do
+          expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+        end
 
-      it "raises an error on a string" do
-        expect { subject.parse('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
-      end
+        it "raises an error on a string" do
+          expect { subject.parse('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
+
+        it "raises an error on a true bool" do
+          expect { subject.parse("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
 
-      it "raises an error on a true bool" do
-        expect { subject.parse("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
+        it "raises an error on a false bool" do
+          expect { subject.parse("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
       end
 
-      it "raises an error on a false bool" do
-        expect { subject.parse("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
+      context "feature flag is disabled" do
+        before do
+          stub_feature_flags(json_wrapper_legacy_mode: false)
+        end
+
+        it "parses an object" do
+          expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+        end
+
+        it "parses an array" do
+          expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+        end
+
+        it "parses a string" do
+          expect(subject.parse('"foo"', legacy_mode: true)).to eq("foo")
+        end
+
+        it "parses a true bool" do
+          expect(subject.parse("true", legacy_mode: true)).to be(true)
+        end
+
+        it "parses a false bool" do
+          expect(subject.parse("false", legacy_mode: true)).to be(false)
+        end
       end
     end
 
-    context "feature flag is disabled" do
-      before do
-        stub_feature_flags(json_wrapper_legacy_mode: false)
-      end
+    describe ".parse!" do
+      context "legacy_mode is disabled by default" do
+        it "parses an object" do
+          expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
+        end
 
-      it "parses an object" do
-        expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
-      end
+        it "parses an array" do
+          expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+        end
 
-      it "parses an array" do
-        expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
-      end
+        it "parses a string" do
+          expect(subject.parse!('"foo"', legacy_mode: false)).to eq("foo")
+        end
+
+        it "parses a true bool" do
+          expect(subject.parse!("true", legacy_mode: false)).to be(true)
+        end
 
-      it "parses a string" do
-        expect(subject.parse('"foo"', legacy_mode: true)).to eq("foo")
+        it "parses a false bool" do
+          expect(subject.parse!("false", legacy_mode: false)).to be(false)
+        end
       end
 
-      it "parses a true bool" do
-        expect(subject.parse("true", legacy_mode: true)).to be(true)
+      context "legacy_mode is enabled" do
+        it "parses an object" do
+          expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+        end
+
+        it "parses an array" do
+          expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+        end
+
+        it "raises an error on a string" do
+          expect { subject.parse!('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
+
+        it "raises an error on a true bool" do
+          expect { subject.parse!("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
+
+        it "raises an error on a false bool" do
+          expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
+        end
       end
 
-      it "parses a false bool" do
-        expect(subject.parse("false", legacy_mode: true)).to be(false)
+      context "feature flag is disabled" do
+        before do
+          stub_feature_flags(json_wrapper_legacy_mode: false)
+        end
+
+        it "parses an object" do
+          expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+        end
+
+        it "parses an array" do
+          expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+        end
+
+        it "parses a string" do
+          expect(subject.parse!('"foo"', legacy_mode: true)).to eq("foo")
+        end
+
+        it "parses a true bool" do
+          expect(subject.parse!("true", legacy_mode: true)).to be(true)
+        end
+
+        it "parses a false bool" do
+          expect(subject.parse!("false", legacy_mode: true)).to be(false)
+        end
       end
     end
-  end
 
-  describe ".parse!" do
-    context "legacy_mode is disabled by default" do
-      it "parses an object" do
-        expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
+    describe ".dump" do
+      it "dumps an object" do
+        expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}')
       end
 
-      it "parses an array" do
-        expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+      it "dumps an array" do
+        expect(subject.dump([{ "foo" => "bar" }])).to eq('[{"foo":"bar"}]')
       end
 
-      it "parses a string" do
-        expect(subject.parse!('"foo"', legacy_mode: false)).to eq("foo")
+      it "dumps a string" do
+        expect(subject.dump("foo")).to eq('"foo"')
       end
 
-      it "parses a true bool" do
-        expect(subject.parse!("true", legacy_mode: false)).to be(true)
+      it "dumps a true bool" do
+        expect(subject.dump(true)).to eq("true")
       end
 
-      it "parses a false bool" do
-        expect(subject.parse!("false", legacy_mode: false)).to be(false)
+      it "dumps a false bool" do
+        expect(subject.dump(false)).to eq("false")
       end
     end
 
-    context "legacy_mode is enabled" do
-      it "parses an object" do
-        expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+    describe ".generate" do
+      let(:obj) do
+        { test: true, "foo.bar" => "baz", is_json: 1, some: [1, 2, 3] }
       end
 
-      it "parses an array" do
-        expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
-      end
+      it "generates JSON" do
+        expected_string = <<~STR.chomp
+          {"test":true,"foo.bar":"baz","is_json":1,"some":[1,2,3]}
+        STR
 
-      it "raises an error on a string" do
-        expect { subject.parse!('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
+        expect(subject.generate(obj)).to eq(expected_string)
       end
 
-      it "raises an error on a true bool" do
-        expect { subject.parse!("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
+      it "allows you to customise the output" do
+        opts = {
+          indent: "  ",
+          space: " ",
+          space_before: " ",
+          object_nl: "\n",
+          array_nl: "\n"
+        }
+
+        json = subject.generate(obj, opts)
+
+        expected_string = <<~STR.chomp
+          {
+            "test" : true,
+            "foo.bar" : "baz",
+            "is_json" : 1,
+            "some" : [
+              1,
+              2,
+              3
+            ]
+          }
+        STR
+
+        expect(json).to eq(expected_string)
       end
+    end
 
-      it "raises an error on a false bool" do
-        expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
+    describe ".pretty_generate" do
+      let(:obj) do
+        {
+          test: true,
+          "foo.bar" => "baz",
+          is_json: 1,
+          some: [1, 2, 3],
+          more: { test: true },
+          multi_line_empty_array: [],
+          multi_line_empty_obj: {}
+        }
+      end
+
+      it "generates pretty JSON" do
+        expected_string = <<~STR.chomp
+          {
+            "test": true,
+            "foo.bar": "baz",
+            "is_json": 1,
+            "some": [
+              1,
+              2,
+              3
+            ],
+            "more": {
+              "test": true
+            },
+            "multi_line_empty_array": [
+
+            ],
+            "multi_line_empty_obj": {
+            }
+          }
+        STR
+
+        expect(subject.pretty_generate(obj)).to eq(expected_string)
+      end
+
+      it "allows you to customise the output" do
+        opts = {
+          space_before: " "
+        }
+
+        json = subject.pretty_generate(obj, opts)
+
+        expected_string = <<~STR.chomp
+          {
+            "test" : true,
+            "foo.bar" : "baz",
+            "is_json" : 1,
+            "some" : [
+              1,
+              2,
+              3
+            ],
+            "more" : {
+              "test" : true
+            },
+            "multi_line_empty_array" : [
+
+            ],
+            "multi_line_empty_obj" : {
+            }
+          }
+        STR
+
+        expect(json).to eq(expected_string)
       end
     end
 
-    context "feature flag is disabled" do
+    context "the feature table is missing" do
       before do
-        stub_feature_flags(json_wrapper_legacy_mode: false)
+        allow(Feature::FlipperFeature).to receive(:table_exists?).and_return(false)
       end
 
-      it "parses an object" do
-        expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+      it "skips legacy mode handling" do
+        expect(Feature).not_to receive(:enabled?).with(:json_wrapper_legacy_mode, default_enabled: true)
+
+        subject.send(:handle_legacy_mode!, {})
       end
 
-      it "parses an array" do
-        expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+      it "skips oj feature detection" do
+        expect(Feature).not_to receive(:enabled?).with(:oj_json, default_enabled: true)
+
+        subject.send(:enable_oj?)
       end
+    end
 
-      it "parses a string" do
-        expect(subject.parse!('"foo"', legacy_mode: true)).to eq("foo")
+    context "the database is missing" do
+      before do
+        allow(Feature::FlipperFeature).to receive(:table_exists?).and_raise(PG::ConnectionBad)
       end
 
-      it "parses a true bool" do
-        expect(subject.parse!("true", legacy_mode: true)).to be(true)
+      it "still parses json" do
+        expect(subject.parse("{}")).to eq({})
       end
 
-      it "parses a false bool" do
-        expect(subject.parse!("false", legacy_mode: true)).to be(false)
+      it "still generates json" do
+        expect(subject.dump({})).to eq("{}")
       end
     end
   end
 
-  describe ".dump" do
-    it "dumps an object" do
-      expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}')
+  context "oj gem" do
+    before do
+      stub_feature_flags(oj_json: true)
     end
 
-    it "dumps an array" do
-      expect(subject.dump([{ "foo" => "bar" }])).to eq('[{"foo":"bar"}]')
-    end
-
-    it "dumps a string" do
-      expect(subject.dump("foo")).to eq('"foo"')
-    end
-
-    it "dumps a true bool" do
-      expect(subject.dump(true)).to eq("true")
-    end
-
-    it "dumps a false bool" do
-      expect(subject.dump(false)).to eq("false")
-    end
+    it_behaves_like "json"
   end
 
-  describe ".generate" do
-    it "delegates to the adapter" do
-      args = [{ foo: "bar" }]
-
-      expect(JSON).to receive(:generate).with(*args)
-
-      subject.generate(*args)
+  context "json gem" do
+    before do
+      stub_feature_flags(oj_json: false)
     end
-  end
 
-  describe ".pretty_generate" do
-    it "delegates to the adapter" do
-      args = [{ foo: "bar" }]
-
-      expect(JSON).to receive(:pretty_generate).with(*args)
-
-      subject.pretty_generate(*args)
-    end
+    it_behaves_like "json"
   end
 end

spec/lib/gitlab/phabricator_import/conduit/response_spec.rb

@@ -30,7 +30,7 @@ RSpec.describe Gitlab::PhabricatorImport::Conduit::Response do
                        body: 'This is no JSON')
 
       expect { described_class.parse!(fake_response) }
-        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected token at/)
+        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected character/)
     end
 
     it 'returns a parsed response for valid input' do