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

test(i): Update mut. tests to make use of mut. system #1853

Merged
merged 12 commits into from
Sep 14, 2023

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Sep 7, 2023

Relevant issue(s)

Resolves #1851

Description

Updates the mutation tests to make use of mutation system.

A handful of collection based tests still exist, as they test stuff that does not yet have support in the new mutation test system (filter, UpdateWithKey(s)). These should be moved later when such support is added.

Similarly, the new mutation system does not yet support txns, so the gql request based txn tests remain as gql tests for now.

#1703 Was linked up to tests in this PR, and #1852 and #1854 were discovered. Fixing them is out of scope here.

Update: It does now fix a bug in the http client collection.Create[Many] functions, as it was easier to fix than to open a ticket.

Recommend reviewing commit by commit, nothing to fancy is going on, but it might make stuff more obvious.

@AndrewSisley AndrewSisley added area/testing Related to any test or testing suite action/no-benchmark Skips the action that runs the benchmark. labels Sep 7, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.7 milestone Sep 7, 2023
@AndrewSisley AndrewSisley requested a review from a team September 7, 2023 18:36
@AndrewSisley AndrewSisley self-assigned this Sep 7, 2023
@AndrewSisley AndrewSisley changed the title test(i): Update mutation tests to make use of mutation system test(i): Update mut. tests to make use of mut. system Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.11% ⚠️

Comparison is base (447a8a7) 70.37% compared to head (e7242e1) 70.26%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1853      +/-   ##
===========================================
- Coverage    70.37%   70.26%   -0.11%     
===========================================
  Files          232      232              
  Lines        24180    24192      +12     
===========================================
- Hits         17016    16998      -18     
- Misses        6003     6029      +26     
- Partials      1161     1165       +4     
Flag Coverage Δ
all-tests 70.26% <0.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
http/client_collection.go 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 447a8a7...e7242e1. Read the comment docs.

@AndrewSisley AndrewSisley force-pushed the 1851-mutation-tests branch 2 times, most recently from c64d1f5 to efde22c Compare September 7, 2023 19:23
@AndrewSisley AndrewSisley requested review from shahzadlone and a team September 8, 2023 16:36
Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks fine for what it does, however I have a non-blocking question, doesn't the use of mutation system over the actual gql queries like we were doing it before now introduce a gap between integration testing a client's actual end-to-end use case?

@AndrewSisley
Copy link
Contributor Author

doesn't the use of mutation system over the actual gql queries like we were doing it before now introduce a gap between integration testing a client's actual end-to-end use case?

Only visually, the mutation queries are still executed by the tests. That loss of visibility is a cost that I am willing to pay for the increased coverage.

@AndrewSisley AndrewSisley force-pushed the 1851-mutation-tests branch 8 times, most recently from b859dc7 to 423decf Compare September 14, 2023 15:03
- TestMutationCreateOneToMany_AliasedRelationNamToLinkFromSingleSide_NoIDFieldError was removed as it is already tested.  The first create has no impact on the outcome of the test.
- TestMutationCreateOneToMany_RelationIDToLinkFromSingleSide_NoIDFieldError was removed for the same reasons.
@AndrewSisley AndrewSisley merged commit c70676e into sourcenetwork:develop Sep 14, 2023
14 checks passed
@AndrewSisley AndrewSisley deleted the 1851-mutation-tests branch September 14, 2023 15:37
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…1853)

## Relevant issue(s)

Resolves sourcenetwork#1851

## Description

Updates the mutation tests to make use of mutation system.

A handful of collection based tests still exist, as they test stuff that
does not yet have support in the new mutation test system (filter,
UpdateWithKey(s)). These should be moved later when such support is
added.

Similarly, the new mutation system does not yet support txns, so the gql
request based txn tests remain as gql tests for now.

sourcenetwork#1703 Was linked up to
tests in this PR, and
sourcenetwork#1852 and
sourcenetwork#1854 were discovered.
Fixing them is out of scope here.

Update: It does now fix a bug in the http client collection.Create[Many]
functions, as it was easier to fix than to open a ticket.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/testing Related to any test or testing suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make existing mutation tests use mutation test system
2 participants