search

hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.07k stars 2.76k forks source link

server/tests: `TestGraphQLQueryBasicCommon` equivalent #7758

Closed sassela closed 2 years ago

sassela commented 2 years ago

Implement the equivalent of TestGraphQLQueryBasicCommon (pytests are here) in the Haskell integration test suite.

sassela commented 2 years ago

This issue involves more of an audit of existing specs than I made clear in the original description, so I'm sorry about any duplicated effort that followed as a result.

Before porting over any more specs from pytest -> hspec, let's please clarify the state of work so far on BasicCommon tests:

  1. which pytests tests have been ported to hspec for all backends*? For example, any from the https://github.com/hasura/graphql-engine/issues/7755 to figure out how we'd share schema setup/teardown
  2. which pytests have been ported to hspec for a single backend as part of https://github.com/hasura/graphql-engine/issues/7757?
  3. which pytests have been ported to hspec for a single backend, but which we can't test in the same way across all backends for any reason?
  4. which pytests from BasicCommon have not yet been ported to hspec at all?

*all backends = Postgres, Citus, BigQuery, SQL Server (exclude MySQL for now)

BasicCommon pytests

test_select_query_author
test_select_query_author_quoted_col
test_select_query_author_with_skip_directive
test_select_query_author_with_include_directive
test_select_query_author_with_skip_include_directive
test_select_query_author_with_wrong_directive_err
test_select_query_where
test_nested_select_query_article_author
test_nested_select_query_deep
test_nested_select_query_where
test_nested_select_query_where_on_relationship
test_select_query_non_tracked_table
test_select_query_col_not_present_err
test_select_query_multiple_columns_arr_fkey
test_select_query_multiple_columns_obj_fkey
test_select_query_author_pk
test_select_query_author_pk_null
test_select_placeholder_err
psibi commented 2 years ago

I have done an initial analysis of the existing hspec test suite: https://gist.github.com/psibi/e3c266c62170cd13aa4724794cc66102

psibi commented 2 years ago

Based on the above gist, I'm summarizing things here. These are the haskell spec that has been implemented which are part of TestGraphQLQueryBasicCommon (from python):

The above haskell suite contains the implementation of the following yamls:

These are the additional tests that are not yet implemented from the TestGraphQLQueryBasicCommon:

We also need to ensure that the above test suite which has been already been implemented - is done for all the three backends of TestGraphQLQueryBasicCommon (right now that's not the case).

sassela commented 2 years ago

@psibi this list LGTM, thanks again for putting it together. For easier accounting next time, would you consider something like adding a code comment for each hspec referencing the pytest it's replacing? Or something more elegant if you can think of it. I think that'd make it easier to remove the corresponding pytests wholesale after this work is done.

psibi commented 2 years ago

@sassela I have a PR ready locally which does these things:

This PR is also implemented on top of the primitive MR which I have already created. I'm delaying opening it until that one is merged, so that the PR review is much easier. (But let me know if you want me to open it anyway).

sassela commented 2 years ago

@psibi thanks for the update! Please go ahead and open the tests PR and consider using https://github.com/hasura/graphql-engine-mono/pull/4263 as the base branch (rather than master) for a clearer diff. The base branch should update automatically to master once the PR's merged.

In any case, I've approved the PR with the hspec primitives so that should be auto-merged soon.

sassela commented 2 years ago

Addressed at https://github.com/hasura/graphql-engine/commit/c9a5d51c70ed6b90ebde4021d77911dbcd95d17b, although there are some remaining BigQuery tests outstanding. @psibi mind creating a tracking issue for the BigQuery tests you're looking into?

psibi commented 2 years ago

@sassela Created here: https://github.com/hasura/graphql-engine/issues/8433 Thanks!