Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change message regressions on 1.1.1 #50

Open
bessey opened this issue Feb 10, 2022 · 2 comments
Open

Change message regressions on 1.1.1 #50

bessey opened this issue Feb 10, 2022 · 2 comments

Comments

@bessey
Copy link
Contributor

bessey commented Feb 10, 2022

Background

Hi, we use the schema comparer library to output breaking change warnings as PR comments, as I'm sure many do. We do not use the rake task directly but rather take the output of GraphQL::SchemaComparator.compare and process it to our liking.

As we have some niche use case customisations, we have extensive testing of the output of our custom comparer.

Problem

In a routine upgrade from 1.0.0 to 1.1.1 (while remaining on GraphQL 1.12.10) while most tests still passed, we saw 2 failures:

The first is that removed arguments are missing their type:

Before: "Argument include_internal: Boolean! was removed from field Query.organisation"
After: "Argument include_internal: was removed from field Query.organisation"

The second is that when field description is changed on an interface which is implemented by another type, that type's name is no longer included in output, instead duplicating the interface's name:

Diff:04:08
@@ -1,7 +1,7 @@04:08
 ## Detected safe changes in your GraphQL schema for `/v1/graphql`04:08
 04:08
 Detected changes when compared against the staging schema @ 12/08/2021:04:08
-- ✅  Field `Organisation.visibleAccountsCount` description changed from `Number of accounts` to `Number of accounts on organisation`04:08
+- ✅  Field `OrganisationalUnit.visibleAccountsCount` description changed from `Number of accounts` to `Number of accounts on organisation`04:08
 - ✅  Field `OrganisationalUnit.visibleAccountsCount` description changed from `Number of accounts` to `Number of accounts on organisation`04:08

in this instance the relevant SDL is:

type Organisation implements OrganisationalUnit {}

interface OrganisationalUnit {
  """
  Number of accounts on organisation
  """
  visibleAccountsCount: Int
}
@bessey bessey changed the title Output regressions on 1.1.1 Change message regressions on 1.1.1 Feb 10, 2022
@swalkinshaw
Copy link
Collaborator

1.1 was a large refactor to use the newer graphql class-based API, so it's not too surprising that a few things fell through the cracks of the test coverage.

Did you want to try contributing these fixes?

@bessey
Copy link
Contributor Author

bessey commented Mar 2, 2022

Yep, I'll give it a go. Was blocked on some other 1.13 bugs but those are out the way now

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

No branches or pull requests

2 participants