Skip to content

test: backfill nodeByUri query testing#2659

Merged
jasonbahl merged 24 commits intowp-graphql:developfrom
justlevine:tests/node-by-uri
Feb 14, 2023
Merged

test: backfill nodeByUri query testing#2659
jasonbahl merged 24 commits intowp-graphql:developfrom
justlevine:tests/node-by-uri

Conversation

@justlevine
Copy link
Copy Markdown
Collaborator

@justlevine justlevine commented Dec 7, 2022

What does this implement/fix? Explain your changes.

This PR backfills NodeByUriTest to cover both untested cases coded for in NodeResolver::resolve_uri(), as well as additional edge cases.

Failing edge cases are marked incomplete.

What's included

  • Cleaned up existing tests; added comments to indicate exactly what $query_vars are being tested for each query; and fixed/expanded testing with/without pretty permalinks.
  • Test user archives
  • Test post format archives
  • Test date archives
  • Test attachment URIs
  • Test non-existent URIs
  • Test posts/pages with a fixed pretty permalink prefix(Querying non-existent single page by URI returns error instead of null #1639)
  • Test URIs with #anchors (URI inputs should sanitize anchor links from uris #1712 )
  • Test post URIs with subpages
  • Test slug conflicts across post types.
  • Test updated URIs.
  • Test hierarchical terms.
  • Test slug conflicts across Post types and Taxonomy terms.
  • Test default term URIs.
  • Test CPTs and Taxonomies with base URIs.
  • Test URI with add_rewrite_rule().

What's not included

  • Comment URIs. Although a comments have a URI, its currently not supported by by WPGraphQL.
  • Using add_rewrite_tag() and add_permastruct(). I'm not familiar enough with these methods to write a test.

Issues uncovered

The following test cases fail (as of WPGraphQL v1.13.7)

These tests have been marked incomplete.

Does this close any currently open issues?

Part of #2366

Any relevant logs, error output, GraphiQL screenshots, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

  • This PR intentionally doesn't attempt to fix any issues with NodeResolver::resolve_uri(), to allow for easy merging and smaller changesets on future PRs.
  • If there are any other edge cases we should check for please let me know in the comments.
  • It's going to take some effort to close all the incomplete tests. While I know there's no precedent for having incomplete tests in this repo, I think we should merge anyway to prevent regressions from other PRs while we work our way through 🎯 Tracking: Resolving nodes by URI #2366.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + PHP 8.0.19)

WordPress Version: 6.1.1

@justlevine justlevine changed the title tests: backfill nodeByUri query testing test: backfill nodeByUri query testing Dec 7, 2022
@justlevine justlevine added scope: tests Developing unit tests, integration tests, and ensuring coverage status: in progress Currently being worked on component: query Relating to GraphQL Queries type: chore Maintenance tasks, refactoring, and other non-functional changes labels Dec 7, 2022
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 7, 2022

Coverage Status

Coverage increased (+3.4%) to 84.804% when pulling 459c8ec on justlevine:tests/node-by-uri into 6ae12df on wp-graphql:develop.

@justlevine justlevine marked this pull request as ready for review December 17, 2022 10:11
@justlevine justlevine requested a review from jasonbahl December 17, 2022 10:32
@justlevine
Copy link
Copy Markdown
Collaborator Author

@jasonbahl I'm going to get started on refactoring work using this as the base branch, but like I wrote in the (updated) description, I dont think we should wait to fix the issues in NodeResolver::resolve_uri() before merging these tests.

@justlevine justlevine added status: in review Awaiting review before merging or closing and removed status: in progress Currently being worked on labels Dec 17, 2022
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 459c8ec and detected 0 issues on this pull request.

View more on Code Climate.

@jasonbahl jasonbahl merged commit fd92397 into wp-graphql:develop Feb 14, 2023
@justlevine justlevine deleted the tests/node-by-uri branch February 15, 2023 17:42
This was referenced Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: query Relating to GraphQL Queries scope: tests Developing unit tests, integration tests, and ensuring coverage status: in review Awaiting review before merging or closing type: chore Maintenance tasks, refactoring, and other non-functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants