Skip to content

Fix Ruby warnings in the GraphQL::Schema::InputObject class tests#5623

Merged
rmosolgo merged 2 commits into
rmosolgo:masterfrom
duffuniverse:fix-ruby-warnings-in-tests
May 14, 2026
Merged

Fix Ruby warnings in the GraphQL::Schema::InputObject class tests#5623
rmosolgo merged 2 commits into
rmosolgo:masterfrom
duffuniverse:fix-ruby-warnings-in-tests

Conversation

@duffuniverse
Copy link
Copy Markdown
Contributor

I ran tests for the GraphQL::Schema::InputObject class with Ruby warnings enabled:

RUBYOPT="-W2" bundle exec rake test TEST=spec/graphql/schema/input_object_spec.rb

/home/andrey/projects/graphql-ruby/spec/graphql/schema/input_object_spec.rb:848: warning: assigned but unused variable - z

Tests passed though 🟢 .

@rmosolgo
Copy link
Copy Markdown
Owner

I agree it'd be good to address this warning, but this change actually changes the semantics of the test. The intention of the test is to confirm that InputObject pattern matching is implemented correctly, and it doesn't match as if it had keys which it doesn't have.

The object in this test doesn't have a z: key, but that in ... pattern would match if there was a z: key.

By modifying the in ... pattern to z: 5, we change the semantics of the test so that it's testing that either 1) input_object has no z: key; or 2) it has a z: key but the value for z: is not 5.

I realize this is splitting hairs but I don't think that reducing the scope of an existing test is the right way to address this warning.

One way to keep the existing test but address the warning would be to use z in the case code:

      in { z: }
        raise "Unexpectedly matched a non-present key, z: #{z.inspect} (in: #{input_object.inspect})"

That would cause the test to fail if z: is present at all, but still please Ruby that z is used. What do you think?

(I would have left it as a code suggestion but I couldn't figure out how to suggest a change to the unmodified line of the test 😖)

@duffuniverse
Copy link
Copy Markdown
Contributor Author

Hmm, that’s an interesting approach - applied

@rmosolgo
Copy link
Copy Markdown
Owner

Thanks for this improvement!

@rmosolgo rmosolgo merged commit 242866a into rmosolgo:master May 14, 2026
12 of 13 checks passed
@duffuniverse duffuniverse deleted the fix-ruby-warnings-in-tests branch May 14, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants