Skip to content

Add unit tests for JsonPrettifier, graphql.Utils, and GraphQLSchemaTo… - #227

Open
abdulhasyyb wants to merge 1 commit into
doyensec:devfrom
abdulhasyyb:add-test-coverage-193
Open

abdulhasyyb wants to merge 1 commit into
doyensec:devfrom
abdulhasyyb:add-test-coverage-193

Conversation

@abdulhasyyb

Copy link
Copy Markdown

…SDL (#193)

Summary

Addresses #193 by adding initial unit test infrastructure and coverage to InQL.

The project currently has no src/test directory or test framework configured, so this PR sets up JUnit 5 (useJUnitPlatform()) and adds test coverage as a starting point.

What's covered

  • JsonPrettifier — valid JSON formatting, nested structures, arrays, non-HTML-escaping, malformed-input fallback behavior, and primitive values
  • inql.graphql.UtilsformatComment (List overload, including line-wrapping), unwrapType, isBuiltInScalarType
  • GraphQLSchemaToSDL — round-trips a schema built via graphql-java's own SchemaParser/SchemaGenerator through schemaToSDL(), covering object types, custom scalars, enums, and interfaces

Scope notes

This is intentionally scoped to classes that don't touch Burp's Montoya API (HttpRequest, UI components, etc.), since those need mocking/fixture work that felt like a separate, larger PR. Happy to follow up with:

  • getGraphQLOperations / other request-parsing functions in graphql.Utils (needs HttpRequest test doubles)
  • GQLQueryPrinter (query generation from schema — core to InQL's scanning feature)

if this approach looks right to you.

Testing

./gradlew test and ./gradlew build both pass locally against dev (JDK 17).

@bartek-doyensec

Copy link
Copy Markdown
Contributor

Hi! Thank you for the first PR. It looks like a solid starting point.

A few things you could clean up here before we merge:

  • You used JUnit BOM 5.10.2, which is quite dated. Can you bump it to the latest 6.1.x before we merge? We're already on Java 17 so that should be fine.
  • JsonPrettifier is basically a Gson wrapper, so a lot of those tests aren't really testing our code. Same with GraphQLSchemaToSDL. Either slim those down to the bits that are actually InQL behavior (HTML escaping off, bad JSON falls back, printer options) or drop them and focus on normalizeGraphQLDocument / isGraphQLDocument instead - those are more worth it.
  • The contains(...) checks are a bit soft. A couple of exact assertions would make regressions easier to catch. And you can delete the comment in UtilsTest about the formatComment(String) overload that isn't there anymore

Thank you!

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