Capture values for all tests, make truly drop-in, refactor #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 6e41533 in 2 minutes and 18 seconds. Click for details.
- Reviewed
112lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4draft 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/expectation_helper_wrapper.rb:289
- Draft comment:
The key '#{RSpec.current_example.location}:#{RSpec.current_example.description}' is constructed for storing test values. Ensure that combining location and description always yields a unique key, especially if similar descriptions appear. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% RSpec.current_example.location includes file path and line number, which is inherently unique. The description is just extra context. Even if two tests had the same description, the location would differentiate them. This is a standard RSpec pattern and the comment seems overly cautious about a non-issue. I could be wrong about how RSpec.current_example.location works in all cases. There might be edge cases I'm not considering. Even if there were edge cases, RSpec's own internals rely on location being unique for test identification. This is a well-established pattern. The comment should be deleted as it raises a speculative concern about a standard RSpec pattern that is known to work reliably.
2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:293
- Draft comment:
Consider simplifying 'passed: result.nil? ? false : true' to a more concise form (e.g. using !!result) for clarity. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. lib/rspec/enriched_json/expectation_helper_wrapper.rb:296
- Draft comment:
Silently rescuing exceptions in the value-capture block might hide serialization issues. Consider logging the error (or at least a minimal debug message) to aid in troubleshooting. - 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% This is a test helper system that captures test values. The rescue block is specifically for value capture errors, which are non-critical - they don't affect test execution or results. Adding logging could create noise in test output. The code already has other places where it handles serialization errors gracefully and silently (like in serialize_value). The silent handling seems intentional for this non-critical feature. I could be underestimating the value of debugging information. There could be scenarios where knowing about serialization failures would help diagnose test issues. The value capture is an optional feature for enriching test output - it's not core functionality. Keeping test output clean and focused is more important than logging non-critical errors. The comment should be removed. The silent error handling is appropriate for this non-critical test enrichment feature.
4. lib/rspec/enriched_json/formatters/enriched_json_formatter.rb:40
- Draft comment:
The key used to look up captured values ('#{notification.example.location}:#{notification.example.description}') must match the one in the expectation wrappers exactly. Also consider clearing the captured values after formatting to prevent potential memory buildup. - 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 first part about key matching seems speculative - we can't verify if this is actually a problem without seeing the expectation wrapper code. The second part about memory cleanup is a valid concern but more of a suggestion than a critical issue. Neither point has strong evidence of being an actual problem in the current code. I might be undervaluing the importance of memory management in test frameworks. The key format mismatch could be a real issue that would cause silent failures. While these concerns are valid, they're speculative without seeing the expectation wrapper code. The memory management suggestion is more of an optimization than a required fix. The comment should be removed as it makes speculative assumptions about key matching and suggests optional optimizations rather than pointing out clear issues.
Workflow ID: wflow_pH43uPc1hHxmniSE
You can customize 
| module RSpec | ||
| module EnrichedJson | ||
| # Storage for all test values (pass or fail) | ||
| @all_test_values = {} |
There was a problem hiding this comment.
Global state (@all_test_values) is used to capture values but isn’t thread‐safe. In parallel runs this might cause race conditions. Consider using a thread-safe structure or proper synchronization.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 269b944 in 1 minute and 59 seconds. Click for details.
- Reviewed
133lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft 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/expectation_helper_wrapper.rb:54
- Draft comment:
Using Oj.dump now always returns a JSON string instead of a Ruby object (as before). Confirm that downstream consumers can handle this type change. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The serialized values are only used internally within this test helper. Looking at how the values are used, they are stored in hashes that eventually become part of error messages or test value storage. Both use cases can handle either Ruby objects or JSON strings equally well. The change to using Oj.dump actually makes the output more consistent since everything will be proper JSON. I could be wrong about how the serialized values are used - there might be other consumers of this method that expect Ruby objects and would break with JSON strings. The method is part of a private Serializer module within a test helper - it's not meant to be used outside this context. The only uses visible in the file are for error messages and test value storage, both of which work fine with JSON strings. The comment should be deleted. The change in return type is intentional and safe within the context of this test helper module.
2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:39
- Draft comment:
The removal of custom truncation and depth/size limits means large or deeply nested values may be fully serialized. Verify this change won’t impact performance or overwhelm output. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that the change won't impact performance or overwhelm output, which is against the rules. It doesn't provide a specific suggestion or ask for a test to be written. Therefore, it should be removed.
3. rspec-enriched_json.gemspec:26
- Draft comment:
Added dependency on 'oj' with '~> 3.16'. Ensure this constraint is compatible with all target environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change and asks the author to ensure compatibility, which violates the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
Workflow ID: wflow_OFO0XsRVlLZEgQD6
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 4a4fd25 in 1 minute and 42 seconds. Click for details.
- Reviewed
53lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft 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/expectation_helper_wrapper.rb:80
- Draft comment:
Using Oj.dump(value.inspect, mode: :compat) for Regexp serialization is a good change; ensure that the quoted string matches expected output compared to the previous to_json call. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:89
- Draft comment:
Replacing the fallback hash's .to_json with Oj.dump(..., mode: :compat) improves consistency; verify that error details remain correctly serialized. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. lib/rspec/enriched_json/formatters/enriched_json_formatter.rb:117
- Draft comment:
Removing the additional .to_json call in safe_serialize avoids double encoding; confirm that the serializer’s JSON output integrates seamlessly with the formatter. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_ehn21hktnP09DHaC
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 86b79d1 in 42 seconds. Click for details.
- Reviewed
166lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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. TODOS.md:1
- Draft comment:
Confirm if removal of TODOS.md is intentional. If it's deprecated or replaced by another tracking system, consider archiving its contents for future reference. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_DoDV3gcxKHPQjmU9
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f3d63dc in 58 seconds. Click for details.
- Reviewed
121lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft 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. CHANGELOG.md:8
- Draft comment:
Changelog entry for v0.6.1 is clear and comprehensive. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. spec/oj_serialization_spec.rb:8
- Draft comment:
Minor whitespace/formatting adjustments; tests remain clear and comprehensive. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. spec/support/regex_test_integration.rb:1
- Draft comment:
Improved spacing in integration tests increases readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_PKbFWVsRFKEIC1yk
You can customize
There was a problem hiding this comment.
Skipped PR review on 4e86c52 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 9f811aa in 1 minute and 24 seconds. Click for details.
- Reviewed
86lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft 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/expectation_helper_wrapper.rb:103
- Draft comment:
Removed matcher-specific instance variable capture inhandle_failuresimplifies error details but may reduce valuable debugging context. Ensure that the expected debugging information is still sufficient for failures. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment suggests that the removal of a specific instance variable might reduce debugging context. It asks to ensure that the debugging information is still sufficient, which is a form of asking the author to double-check something. This violates the rule against asking the author to ensure behavior is intended or to double-check things.
2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:144
- Draft comment:
The complete removal of theextract_matcher_specific_datamethod eliminates detailed matcher internals from failure data. Confirm that capturing only basic expected/actual values meets the debugging needs. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_zKjWpryMUa6sUAxp
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 81d5f87 in 1 minute and 22 seconds. Click for details.
- Reviewed
30lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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/expectation_helper_wrapper.rb:119
- Draft comment:
Use RSpec.configuration.color_enabled? dynamically instead of a hardcoded false; ensures diff colors match config. This aligns with best practices. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. spec/support/regex_test_integration.rb:1
- Draft comment:
Removal of regex test integration file appears intentional. Confirm that Regexp serialization tests are covered elsewhere. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_1d7nVeZGX4sBoTma
You can customize
raghubetina
changed the title
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed d3bad71 in 35 seconds. Click for details.
- Reviewed
44lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4draft 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. .github/workflows/ci.yml:29
- Draft comment:
Good: Newline added at end-of-file ensures POSIX compliance. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. lib/rspec/enriched_json/expectation_helper_wrapper.rb:45
- Draft comment:
Removed the unused parameter 'depth' from serialize_value. Ensure no external calls need this parameter. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment suggests ensuring that no external calls need the removed parameter. This is a request for confirmation, which violates the rules. However, the comment also points out the removal of an unused parameter, which is a valid observation. The part about ensuring no external calls need this parameter should be removed.
3. spec/diff_info_spec.rb:102
- Draft comment:
Removed extraneous blank line at end of file. Clean formatting. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. spec/verify_custom_message_spec.rb:41
- Draft comment:
Removed extra trailing newline at end-of-file for cleaner formatting. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_XKd77Dm6d4Em8Opm
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6fbd240 in 28 seconds. Click for details.
- Reviewed
70lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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. analyze_builtin_json.rb:1
- Draft comment:
Removed demonstration script. If this removal is intentional, ensure that any documentation or examples referring to this file are updated. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_JKp8MiDGG3RkXGnb
You can customize
raghubetina
force-pushed
the
rb-capture-values-for-passing
branch
from
6fbd240 to
00800be
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 9d22c6a in 29 seconds. Click for details.
- Reviewed
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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. README.md:3
- Draft comment:
The added badges (CI, Gem Version, and Ruby Style Guide) enhance the README by providing quick project status and style compliance info. The URLs look correct; just ensure they're maintained with any future changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_STnk0dC71YZAKz4G
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 3935e68 in 47 seconds. Click for details.
- Reviewed
102lines of code in6files - Skipped
0files when reviewing. - Skipped posting
6draft 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. .github/workflows/ci.yml:14
- Draft comment:
Good update: Adding Ruby 3.4 to the matrix and using checkout@v4 ensures broader CI coverage and current best practices. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. bin/console:4
- Draft comment:
Consistent quoting: Changed require statements from single to double quotes for style consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. bin/rake:4
- Draft comment:
Style update: Uniform quoting in require and load statements improves readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. bin/rspec:4
- Draft comment:
Minor style fix: Consistent use of double quotes in require and load statements. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. bin/rubocop:4
- Draft comment:
Consistent quoting applied in rubocop script for clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. bin/setup:8
- Draft comment:
Stylistic improvement: Using double quotes for file paths and strings ensures consistency throughout the script. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_m0AVWFjo9GBKdxjr
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 819d2ea in 1 minute and 16 seconds. Click for details.
- Reviewed
80lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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. README.md:102
- Draft comment:
The PR adds trailing periods to bullet and numbered list items for consistent punctuation. Confirm this stylistic change aligns with your documentation guidelines. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm a stylistic change, which violates the rule against asking for confirmation of intention. It does not provide a specific code suggestion or ask for a test to be written. Therefore, it should be removed.
Workflow ID: wflow_Acs5C0S1khxCBq0s
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f1a6427 in 49 seconds. Click for details.
- Reviewed
38lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft 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. .github/workflows/ci.yml:14
- Draft comment:
Ruby matrix now targets only versions >=3.2, which aligns with the gemspec update. Ensure this drop of older versions is clearly documented. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. lib/rspec/enriched_json/version.rb:5
- Draft comment:
Version bumped to 0.8.0. Confirm that changelog and documentation reflect the breaking changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. rspec-enriched_json.gemspec:23
- Draft comment:
Ruby version requirement updated to '>= 3.2'. Ensure this breaking change is well documented for users. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_xFkgHgekI1hkJHhU
You can customize
- Add PositiveHandlerWrapper and NegativeHandlerWrapper modules - Hook into RSpec's expectation handlers to capture values for all tests - Store captured values in RSpec::EnrichedJson.all_test_values - Modify formatter to include captured values for passing tests in JSON output - Now all tests (pass or fail) have details with expected/actual values
Register an after(:suite) hook to clear captured test values when the test suite finishes. This prevents memory accumulation when running multiple test suites in the same process or when dealing with large test suites. - Add RSpec.configure block in install\! method - Register after(:suite) hook to call clear_test_values - Add tests to verify cleanup functionality - Include demo script showing memory cleanup in action
- Fix redundant ternary operators in expectation_helper_wrapper.rb - Add StandardRB usage instructions to README Development section - Ensure all files follow Ruby Standard Style conventions
Replace location:description key format with RSpec's built-in example ID which includes the full hierarchy (e.g., "./file.rb[1:2:1:1]"). This ensures unique keys for: - Shared examples with same descriptions - Dynamically generated tests - Same descriptions in different contexts Also improved value capture to handle failures by capturing before calling super method which might raise.
- Fix key mismatch between storage (example.id) and retrieval - Move memory cleanup from after(:suite) to formatter's close method to ensure values are available when formatting JSON output - Add comprehensive test coverage for passing test value capture - Add integration tests verifying JSON output includes passing test details - Update memory cleanup approach to prevent clearing values too early The feature now correctly captures and outputs expected/actual values for both passing and failing tests in the JSON formatter output.
- Only serialize expected and actual values with Oj - Keep other fields (passed, negated, matcher_name, etc.) as regular JSON values - Update integration tests to match client usage pattern (two-step deserialization) - Document limitations with regex deserialization due to security settings - Remove problematic hanging test file This ensures proper JSON output where boolean fields remain booleans and only complex Ruby objects in expected/actual use Oj serialization.
The simple_passing_test_spec was testing internal implementation details of value capture. The integration tests provide better coverage by testing the actual JSON output that clients will consume.
The test was capturing its own expectations, causing failures. Simplified to just verify the clear functionality works.
After exploring multiple approaches to serialize Regexp objects: 1. First tried Oj.register_odd_raw with custom RegexpWrapper 2. Discovered Oj serializes the wrapper class itself, not just output 3. Attempted direct JSON structure with _regexp_source/_regexp_options 4. Found Oj embeds our JSON as a string within its own structure Settled on the simplest solution: use Regexp#inspect which produces human-readable format like /hello/i. This approach: - Works consistently across all contexts - Provides clear visual representation of patterns and flags - Avoids complex nested JSON structures - Maintains compatibility with existing serialization Also includes: - Comprehensive test coverage for regex serialization - Cleanup of obsolete test files - StandardRB formatting fixes - Updated .gitignore to exclude oj directory
Initially attempted to use Oj's register_odd_raw feature to customize
Regexp serialization, but discovered it still wraps output in Oj's
internal structure. After exploring various approaches, settled on a
simple solution: serialize Regexp objects using their inspect format
(e.g., "/hello/i").
This approach provides:
- Human-readable regex representations in test output
- Simpler client-side handling (no special parsing needed)
- Clear indication of regex flags (i, m, x)
- Consistent with Ruby's standard regex display format
The implementation checks if a value is a Regexp and returns
value.inspect.to_json, bypassing Oj's default object serialization
which only shows {"^o":"Regexp"} without pattern data.
Also includes:
- Updated fuzzy_match_poc client to handle simplified format
- Added test coverage for regex serialization
- Cleaned up experimental test files
- Fixed StandardRB formatting violations
The safe_serialize method was calling .to_json on the result of serialize_value, which already returns a JSON string. This caused strings to be double-encoded, resulting in escaped newlines (\n) and quotes in the HTML output. Removed the redundant .to_json call to fix the display issue where multiline content was showing escaped characters instead of proper formatting. Also updated fuzzy_match_poc to detect regex patterns in the format /pattern/flags and display them appropriately.
Remove tests for standard Oj functionality and focus only on our special cases: - Regexp serialization using inspect format - Fallback behavior when Oj.dump fails Also remove unnecessary demo files and simplify fuzzy_match_poc regex detection since we now serialize regexes as simple strings.
Remove external dependencies and build artifacts: - Remove external gem directories (diffy/, fuzzy_match_poc/, oj/, rspec/, super_diff/) These were copied in for AI context but shouldn't be in the repo - Remove built gem file (rspec-enriched_json-0.5.0.gem) - Remove JSON output test files - Remove all .DS_Store files - Remove .claude/ directory - Remove Gemfile.lock (already in .gitignore) - Remove unused spec/support/regex_test_spec.rb Update .gitignore: - Add .claude/ for Claude IDE settings - Add generic *.json pattern for test output files This significantly reduces repository size and removes confusion about which files are part of the gem versus external dependencies.
Future tasks are better tracked as GitHub issues rather than in a markdown file. Many items were already completed and the remaining ones can be converted to issues as needed.
- Document all changes made in this branch including passing test capture, memory management, Regexp serialization, and bug fixes - Fix StandardRB style violations in test files
- Add special handling for BePredicate and Has matchers to capture true/false values instead of the actual object and nil - For predicate matchers, expected is true/false based on positive/negative expectation, and actual is the result of calling the predicate method - Remove redundant serialization in formatter that was causing double-encoding - Delete unnecessary safe_structured_data and safe_serialize methods - Update integration tests to expect single-encoded values - Add comprehensive tests for predicate matcher value capture
- Delete extract_matcher_specific_data method and all related code - Simplify handle_failure by removing matcher data merge - Keep only the essential data: expected, actual, matcher_name, and diffable - This reduces complexity since the client doesn't use matcher-specific data
Since only RaiseError and OperatorMatcher don't respond to diffable?, and they should default to false anyway, we can simply use: matcher.respond_to?(:diffable?) && matcher.diffable? This removes unnecessary complexity and makes the code more direct.
These tests were checking our old logic for determining diffability which we removed in favor of trusting the matcher's diffable? method.
- Remove excessive comments and documentation - Streamline code structure without changing functionality - Fix CLAUDE.md reference to demo.rb (was demo_all_failures.rb) - Clean up formatting and reduce verbosity
- Remove edge_cases_spec.rb (was testing Oj serialization behavior) - Remove safety_limits_spec.rb (was testing non-existent features) - Remove regex_serialization_spec.rb (redundant with oj_serialization_spec) - Fix integration test to expect values for passing tests - Remove problematic nested failing examples from negated_matcher_spec - Update formatter test to reflect current behavior All tests now pass without false failures that would break CI.
- Bump version to 0.7.0 for new features - Update CHANGELOG with latest changes - Fix README to remove non-existent performance limits - Add GitHub Actions CI workflow - Fix StandardRB style violations - Document new features (negated flag, passing test capture)
- No longer removes "Diff:" section from exception messages - Makes the gem a true drop-in replacement for RSpec's JSON formatter - Consumers still get structured data in details but messages are unchanged - Update CHANGELOG to note this breaking change - Remove tests that were checking for diff removal
- Use RSpec.configuration.color_enabled? instead of hardcoding false - Allows consumers to get ANSI colored diffs if they enable color - Still typically false for JSON formatter but respects user preference
Add CI build status, gem version, and code style badges to provide at-a-glance project health indicators.
- Change gem name from single to double quotes - Maintain consistency with StandardRB style guide
- Add Ruby 3.4 to test matrix - Update actions/checkout from v3 to v4
Ensure all list items that are complete sentences end with periods for consistency and improved readability.
Increase minimum Ruby version requirement to 3.2. Update CI to test only Ruby 3.2+. Bump version to 0.8.0 for breaking change.
raghubetina
force-pushed
the
rb-capture-values-for-passing
branch
from
f1a6427 to
233d972
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 233d972 in 32 seconds. Click for details.
- Reviewed
8lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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. spec/support/.gitkeep:1
- Draft comment:
Consider adding a trailing newline at the end of the file for consistency with POSIX standards. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_NfmHgpH0Grl8wS5T
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f80708b in 1 minute and 7 seconds. Click for details.
- Reviewed
17lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft 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. .github/workflows/ci.yml:30
- Draft comment:
Ensure the COVERAGE env var is used by the test suite to trigger coverage output. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. .github/workflows/ci.yml:33
- Draft comment:
Consider adding a check for the existence of the 'coverage/' directory before uploading to avoid potential errors if no coverage report is generated. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. .github/workflows/ci.yml:34
- Draft comment:
Verify that using 'actions/upload-artifact@v4' is intentional; current best practice is often to use a stable version (e.g. v3). - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_TCOhycoy7VPdEMDl
You can customize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR represents a significant evolution of the rspec-enriched_json gem, expanding its capabilities while simplifying its implementation.
The journey involved several pivots and discoveries about RSpec's internals.
Key Changes
The Journey
Challenges Faced
Tracks Explored and Abandoned
Design Decisions
Technical Details
Breaking Changes
Important
Enhance
rspec-enriched_jsonby capturing values for all tests, using Oj for serialization, and ensuring a true drop-in replacement with improved CI/CD and documentation.expectation_helper_wrapper.rb.negatedflag to detectnot_to/to_notusage.expectation_helper_wrapper.rbfor better performance and circular reference handling.enriched_json_formatter.rb.ci.ymlfor automated testing across Ruby versions 3.2, 3.3, 3.4.version.rb.This description was created by
for f80708b. You can customize this summary. It will automatically update as commits are pushed.