Fixes serialization of procs

[swaathi]

Problem

Procs would be dumped to JSON (Oj) without being serialized. This would cause output issues.

Solution

Simple procs are called as part of the serialization workflow.

What used to look like before,

{
  "exception": {
    "class": "RSpec::EnrichedJson::EnrichedExpectationNotMetError",
    "message": "\nexpected: \"Hello!\"\n     got: #<Proc:0x000000012917a5f0 /var/folders/c5/717zkj_j50g1hdbnsb06z3rh0000gn/T/demo_test20250730-63756-o2wass.rb:44>\n\n(compared using ==)\n"
  },
  "details": {
    "expected": "\"Hello!\"",
    "actual": "{\"^o\":\"Proc\",\"^i\":1}",
    "original_message": null,
    "matcher_name": "RSpec::Matchers::BuiltIn::Eq",
    "diffable": true,
    "negated": false
  }
}

Now looks like this,

{
  "exception": {
    "class": "RSpec::EnrichedJson::EnrichedExpectationNotMetError",
    "message": "\nexpected: \"Hello!\"\n     got: #<Proc:0x000000012514c410 /var/folders/c5/717zkj_j50g1hdbnsb06z3rh0000gn/T/demo_test20250730-66617-k5sedp.rb:44>\n\n(compared using ==)\n"
  },
  "details": {
    "expected": "\"Hello!\"",
    "actual": "Hello, world!",
    "original_message": null,
    "matcher_name": "RSpec::Matchers::BuiltIn::Eq",
    "diffable": true,
    "negated": false
  }
}

---

Important

Fixes Proc serialization by calling them during JSON serialization, adds tests, and updates version to 0.8.2.

• Behavior:
  • Proc objects are now called during serialization in serialize_value() in expectation_helper_wrapper.rb.
  • Adds test case for Proc serialization in oj_serialization_spec.rb.
  • Adds example for Proc equality matcher in demo.rb.
• Version:
  • Update version to 0.8.2 in version.rb.

^{This description was created by for a6cf1a7. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to d44f8a2 in 1 minute and 23 seconds. Click for details.

• Reviewed 65 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. demo.rb:64

• Draft comment:
  The new test "eq with proc" compares a proc (which, when called, returns 'Hello, world!') to the string 'Hello!'. If this is intended to demonstrate failure enrichment, consider clarifying the intent with a comment so readers aren’t confused by the mismatch.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:48

• Draft comment:
  In the Serializer#serialize_value method, the Proc branch directly calls value.call. Consider checking the proc's arity (or adding documentation) to ensure it’s only used for parameterless procs, since calling a proc with arguments unintentionally could raise errors.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code is in a test helper that serializes values for enriched test output. The Proc handling is likely meant to evaluate lazy values during serialization. Since this is test code, any Proc arity mismatch would fail the test immediately and be obvious. The rescue block on line 53 would also catch any argument errors. The comment seems overly cautious for test tooling. I could be underestimating the impact of Proc errors in test infrastructure. Bad error messages here could make test failures harder to debug. While better error messages would be nice, the existing error handling will catch and wrap any Proc issues. The current simple implementation is adequate for test tooling. The comment raises a valid point but the suggested change isn't necessary given the context and error handling. The comment should be removed.

3. spec/oj_serialization_spec.rb:47

• Draft comment:
  The test for Proc serialization cleanly verifies that a simple proc is called and its result is returned. Consider adding additional tests for cases where a proc returns a non‐string (or nil) to ensure consistent behavior across different return types.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_IzLZHdegEgbmPcLa

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[jelaniwoods]

Thanks for looking at this!

I wrote a test for this that captured the behavior that happens in firstdraft / Learn codeblocks.

On main the test passes and the "actual" value is the Proc-like String, but on this branch the test fails and the "actual" value is nil. This makes me think this change isn't doing the right thing, since the test fails and the "actual" output is inaccurate.

You can see the spec I wrote here. Could you confirm if this is an issue?

[swaathi]

Nice spec! I was wondering though if this would ever be our use-case:

expect { require_relative("#{Dir.pwd}/spec/support/code.rb") }.to output(/Hello, world!/).to_stdout

We tend to use run_codeblock and save the output to a variable. Then use the variable in a eq matcher.

I tested with the TodoList example and it looked good.

I'll have a look at this scenario anyways. I wonder if it would need a different fix.

[swaathi]

Tried this on Learn which doesn't have this gem installed and noticed that it errors out there as well. I don't think the spec tests this change.

[swaathi]

I updated your spec to more closely resemble the spec on prod.

require "spec_helper"

RSpec.describe "Special serialization cases" do
  describe "stdout actual detection" do
    it "marks string comparisons as diffable" do
      test_content = <<~RUBY
        class TodoList
          attr_accessor :tasks

          def initialize
            self.tasks = []
          end

          def add_task(task)
            tasks.push(task)
          end

          def display_tasks
            pp "No tasks in the list" if tasks.empty?

            tasks.each do |task|
              puts task
            end
          end

          def remove_task(task)
            if tasks.include?(task)
              tasks.delete(task)
              self.tasks = tasks
            else
              puts "Task not found"
            end
          end
        end

        RSpec.describe "stdout" do
          it "matches output" do
            todo_list = TodoList.new
            todo_list.add_task("Buy groceries")
            expect { todo_list.display_tasks }.to output("Buy eggs\n").to_stdout
          end
        end
      RUBY

      p output = run_formatter_with_content(test_content)

      puts "====" * 3
      p output["examples"].first["details"]["actual"]
      p output["examples"].first["details"]["expected"]
      puts "====" * 3
      puts "\n\n\n" * 3
      expect(output["examples"].first["details"]["actual"]).to match("Hello, World!")
    end
  end
end

Output looks good:

[jelaniwoods]

I updated your spec to more closely resemble the spec on prod.

When this spec passes, the "actual" is still the Proc:

In standup we decided that we should just not display the "actual" for this matcher, so we don't need to spend more time on this. I think the change to hide "actual" on passing block output specs needs to be made in rspec-html_messages. I'll work on that today.

[swaathi]

Hm maybe I misunderstood during standup but I think that we should show output when using an output matcher. Why should we not display it?

[jelaniwoods]

Hm maybe I misunderstood during standup but I think that we should show output when using an output matcher. Why should we not display it?

I thought we decided to not display the "actual" results when a test passes for this matcher because by default, the matcher captures Nil or "" for the output which would be confusing/misleading if the file actually outputs something. Fixing this would require special casing the matcher in this gem I recall Raghu saying we don't want to do.

[swaathi]

I thought we decided to not display the "actual" results when a test passes for this matcher because by default, the matcher captures Nil or "" for the output which would be confusing/misleading if the file actually outputs something.

We do not use this matcher to capture output from the code file. We use run_codeblock for that.

The cases where we do use this matcher is to capture output from a method from the code file. Similar to the examples here with "print".

Since we use it with puts (like in the TodoList example above), I think we should show the output (which is now captured in the "actual" object). This is similar to what we do with eq when the spec passes. Technically there is no output, but we still show "actual".

I created a test that looks like this,

require "rspec"

RSpec.describe "output matcher" do
  it "does not capture output when requiring a file" do
    expect { require_relative("#{Dir.pwd}/spec/support/code.rb") }.to output(/Hello, world!/).to_stdout
  end

  it "captures output from stdout" do
    expect { puts "hi" }.to output("bye").to_stdout
  end
end

Added it to the root directory on this repo and used bundle exec rspec (like in demo.rb):

➜ bundle exec rspec stdout_demo.rb

Randomized with seed 57066

output matcher
Hello, world!
  does not capture output when requiring a file (FAILED - 1)
hi
  captures output from stdout (FAILED - 2)

Failures:

  1) output matcher does not capture output when requiring a file
     Failure/Error: raise EnrichedExpectationNotMetError.new(e.message, details)
     
       expected block to output /Hello, world!/ to stdout, but output nothing
       Diff:
       @@ -1 +1 @@
       -/Hello, world!/
       +""
       
     # ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:93:in 'RSpec::EnrichedJson::ExpectationHelperWrapper#handle_failure'
     # ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:185:in 'RSpec::EnrichedJson::PositiveHandlerWrapper#handle_matcher'
     # ./stdout_demo.rb:5:in 'block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     #   expected block to output /Hello, world!/ to stdout, but output nothing
     #   Diff:
     #   @@ -1 +1 @@
     #   -/Hello, world!/
     #   +""
     #   
     #   ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:73:in 'RSpec::EnrichedJson::ExpectationHelperWrapper#handle_failure'

  2) output matcher captures output from stdout
     Failure/Error: raise EnrichedExpectationNotMetError.new(e.message, details)
     
       expected block to output "bye" to stdout, but output "hi\n"
       Diff:
       @@ -1 +1 @@
       -bye
       +hi
       
     # ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:93:in 'RSpec::EnrichedJson::ExpectationHelperWrapper#handle_failure'
     # ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:185:in 'RSpec::EnrichedJson::PositiveHandlerWrapper#handle_matcher'
     # ./stdout_demo.rb:9:in 'block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     #   expected block to output "bye" to stdout, but output "hi\n"
     #   Diff:
     #   @@ -1 +1 @@
     #   -bye
     #   +hi
     #   
     #   ./lib/rspec/enriched_json/expectation_helper_wrapper.rb:73:in 'RSpec::EnrichedJson::ExpectationHelperWrapper#handle_failure'

Finished in 0.00967 seconds (files took 0.34253 seconds to load)
2 examples, 2 failures

Failed examples:

rspec ./stdout_demo.rb:4 # output matcher does not capture output when requiring a file
rspec ./stdout_demo.rb:8 # output matcher captures output from stdout

Randomized with seed 57066

[swaathi]

When this spec passes, the "actual" is still the Proc:

Apologies, I missed this. I am not able to reproduce this. The actual in this example does contain the output string. I've highlighted it in the image.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed f75641a in 23 seconds. Click for details.

• Reviewed 12 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. lib/rspec/enriched_json/version.rb:5

• Draft comment:
  VERSION updated from 0.8.0 to 0.8.1; patch bump seems appropriate for a serialization fix.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_OWY8jKunj5ANXUdY

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[jelaniwoods]

When this spec passes, the "actual" is still the Proc:

Apologies, I missed this. I am not able to reproduce this. The actual in this example does contain the output string. I've highlighted it in the image.

@swaathi When I run this in the Lambda function locally with the playground, I still see the Proc in the "actual" response when the test passes.

2025-08-01.10-20-06.mp4

Note: I updated the Gemfile to use gem "rspec-enriched_json", "~> 0.8.0"

[swaathi]

Hashed this over call with Jelani. This PR works as expected. Will be merging this tomorrow.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed a6cf1a7 in 21 seconds. Click for details.

• Reviewed 12 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. lib/rspec/enriched_json/version.rb:5

• Draft comment:
  Ensure the version bump to 0.8.2 is accompanied by appropriate changelog updates. Also verify that this update complies with the release strategy.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_H8WUXmm47TaiyUyw

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}