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: Convert explain dagscan tests to new explain setup #1474

Merged

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented May 10, 2023

Relevant issue(s)

Description

Continue converting explain tests to the new explain setup before we can integrate the entire setup to the new action based setup. #953 Has a lot more detail on the entire plan.

  • Fixes test names and some fixtures for the limit test.
  • This PR converts all the default dagscan explain tests to the new explain setup.
  • Converts a wrong usage explain test case.

and change the fixtures to only contain the `typeIndexJoin` part so we
can use it across different patterns easily, without making new fixtures
all the time.
@shahzadlone shahzadlone added area/testing Related to any test or testing suite code quality Related to improving code quality labels May 10, 2023
@shahzadlone shahzadlone added this to the DefraDB v0.5.1 milestone May 10, 2023
@shahzadlone shahzadlone self-assigned this May 10, 2023
@shahzadlone shahzadlone force-pushed the test/refactor-dagscan-explain branch 2 times, most recently from fa3b409 to d45372f Compare May 10, 2023 08:22
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #1474 (d45372f) into develop (5df70af) will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1474      +/-   ##
===========================================
+ Coverage    72.11%   72.23%   +0.12%     
===========================================
  Files          185      185              
  Lines        18226    18226              
===========================================
+ Hits         13143    13166      +23     
+ Misses        4040     4023      -17     
+ Partials      1043     1037       -6     

see 7 files with indirect coverage changes

@shahzadlone shahzadlone requested a review from a team May 10, 2023 08:34
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM :) Thanks for this, and breaking it up into multiple PRs (and commits)

@shahzadlone shahzadlone merged commit 053a08c into sourcenetwork:develop May 10, 2023
@shahzadlone shahzadlone deleted the test/refactor-dagscan-explain branch May 10, 2023 14:52
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…rk#1474)

## Relevant issue(s)
- Part of sourcenetwork#953
- Resolves sourcenetwork#1473

## Description
Continue converting explain tests to the new explain setup before we can
integrate the entire setup to the new action based setup. sourcenetwork#953 Has a lot
more detail on the entire plan.

- Fixes test names and some fixtures for the limit test.
- This PR converts all the default dagscan explain tests to the new
explain setup.
- Converts a wrong usage explain test case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Related to any test or testing suite code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert dagscan explain tests to new explain setup.
2 participants