Problem/Motivation

We currently have no automated test coverage. Let's make sure to add sufficient coverage so maintenance work (like #3589522: [PP-1] Support GraphQL 5) can be done without having to do extensive manual testing.

Issue fork graphql_address-3590338

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pfrenssen created an issue. See original summary.

pfrenssen opened merge request !16

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Category: Feature request » Task
Priority: Normal » Major

Very much in favour of this, I'll wait until it's NR to have a proper look.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Category: Task » Feature request
Priority: Major » Normal
Status: Active » Needs review

Thanks! I plan to work next week on #3589522: [PP-1] Support GraphQL 5 and I want to use these tests as a basis for the refactoring.

Disclosure: I wrote the tests using AI assistance, but looked through the generated code and made changes and improvements so it is aligned with Drupal best practices.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented
Category: Feature request » Task
Priority: Normal » Major
Status: Needs review » Needs work

Just a couple of queries on the MR :)

Also those issue metadata changes were deliberate. Tests aren't a feature, this is a task; and as the maintainer I'm calling it Major as it's blocking other work.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs work » Needs review

Thanks for reviewing! I answered the question and implemented the requested change.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
commented

Thanks very much for that @pfrenssen :)

I would mark this RTBC but instead I'm turning the tables: I have added some test coverage, if you could check it over I'd appreciate it. If you're happy, mark this RTBC and I'll commit.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Needs review » Reviewed & tested by the community

Thanks very much @darvanen for filling the gaps!

I had 2 small remarks: we can convert PHPUnit annotations to attributes, and there was a spelling issue flagged. I have already fixed it. Looking good!

pfrenssen’s picture

pfrenssen
Location Sofia
commented

I just realize now I had earlier on reverted the issue metadata, that was unintentional.