Support new parser

[Sam-Killgallon]

The new default parser for GraphQL Ruby doesn't call the lex step, but the tests were expecting the lexing_duration to be non-zero. This updates the tests to to match the new behaviour of:

• Recording the lexing_duration for GraphQL Ruby versions less than 2.2
• Recording the lexing_duration for GraphQL Ruby versions of 2.2+ if they are using the C parser (which does call lex)
• lexing_duration being 0 for GraphQL Ruby versions of 2.2+ if they are not using the C parser

I also encountered a bug with tests leaking state due to the pre_context not being reset in all cases.

Repro command:

BUNDLE_GEMFILE=gemfiles/graphql_2.0.gemfile ruby -Itest:lib -Itest -Ilib -e 'require "./test/unit/graphql/metrics/integration_test.rb" ; require "./test/unit/graphql/metrics/trace_integration_test.rb"' -- --seed 5488  -n "/^(?:GraphQL::Metrics::IntegrationTest#(?:test_skips_analysis,_if_the_query_is_semantically_invalid|test_extracts_metrics_from_queries,_as_well_as_their_fields_and_arguments_\(when_using_Query#result\)))$/"

This was caused by the the first test running:

query = GraphQL::Query.new(
  SchemaWithFullMetrics,
  'HELLO',
)

analysis_results = GraphQL::Analysis::AST.analyze_query(query, [SimpleAnalyzer]).first

and the second test running

query = GraphQL::Query.new(
  SchemaWithFullMetrics,
  kitchen_sink_query_document,
  variables: { 'postId': '1', 'titleUpcase': true },
  operation_name: 'PostDetails',
  context: {}
)
result = query.result.to_h

Test 1 triggered the following trace steps:

analyze_query
parse
validate

pre_context is only reset on the analyze_multiplex step, meaning that some data still existed on the following test. This also bypassed the execute_multiplex step so multiplex_start_time doesn't get set to anything, but lexing_duration does get set to 0

When Test 2 ran the tracer we do:

execute_multiplex
parse
validate
analyze_multiplex
analyze_query
execute_query

and hit the check for lexing_duration it was already 0, not nil which meant we didn't overwrite the lexing_start_time_offset causing it to stay at nil, even though we did have a value available.

[Sam-Killgallon]

I added a cron here to surface CI failures regularly as GraphQL Ruby changes, rather than only discovering them when making an unrelated PR

[ravangen]

Would this trigger a notification? Or just a badge refresh on README (which looks broken to me)

[Sam-Killgallon]

https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/notifications-for-workflow-runs A notification but only to the creator of the workflow.

Which badge in the readme are you referring to? I don't see one for this repo.

EDIT: Ah I misunderstood you mean that the badge is currently broken, which is why I can't see it

[ravangen]

Would this trigger a notification? Or just a badge refresh on README (which looks broken to me)

[swalkinshaw]

Wondering if we should just skip all this conditional for lexing/parsing and default the durations to 0.0 to start with and fallback to multiplex_start_time if neither of the start times exist.

Then the test helper wouldn't be needed either?

[swalkinshaw]

You can probably ignore this for now tbh, let's just focus on fixing the immediate issue

[swalkinshaw]

You can probably ignore this for now tbh, let's just focus on fixing the immediate issue