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: Add DB/Node Restart tests #1504

Merged

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #1082

Description

Adds DB/Node Restart tests. This will be required by Lens (e.g. to make sure migrations are not lost on restart). IIRC I think this also found #1482 (also fixed by Fred).

Fred was a massive help here and fixed the difficult bits when I flaked out on Friday :) Any commit that lists him as an author are his, I am only tagged in them having rebased them.

Commits are messy as anything as I wanted to preserve the level of credit due to Fred, and I'd suggest reviewing this PR at PR level as cleanup has not been squashed into commits introducing broken/messy code.

I will merge this when approved, whether that is 0.5.1 or 0.6 - it introduces no breaking (or risky) changes to the production code.

@AndrewSisley AndrewSisley added area/testing Related to any test or testing suite action/no-benchmark Skips the action that runs the benchmark. labels May 15, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.5.1 milestone May 15, 2023
@AndrewSisley AndrewSisley requested a review from a team May 15, 2023 22:21
@AndrewSisley AndrewSisley self-assigned this May 15, 2023
@AndrewSisley AndrewSisley force-pushed the 1082-test-restart-with-fred-2 branch from 1f4c45b to 393acac Compare May 15, 2023 22:22
@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Merging #1504 (3f63b84) into develop (db4b853) will decrease coverage by 0.10%.
The diff coverage is 40.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1504      +/-   ##
===========================================
- Coverage    72.16%   72.06%   -0.10%     
===========================================
  Files          185      185              
  Lines        18239    18260      +21     
===========================================
- Hits         13162    13159       -3     
- Misses        4037     4056      +19     
- Partials      1040     1045       +5     
Impacted Files Coverage Δ
node/node.go 61.09% <30.76%> (-1.51%) ⬇️
net/peer.go 48.97% <57.14%> (+0.17%) ⬆️

... and 4 files with indirect coverage changes

@AndrewSisley AndrewSisley force-pushed the 1082-test-restart-with-fred-2 branch from 393acac to e1f9703 Compare May 15, 2023 22:34
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM. Of course I'm biased here so I understand if you want to wait for someone else's approval.

@AndrewSisley
Copy link
Contributor Author

LGTM. Of course I'm biased here so I understand if you want to wait for someone else's approval.

😆 I thought I fixed the change detector earlier but hadn't - so the other have at least until then 😁

Looks like this is somehow the first new test not either in a new package, or a P2P test, since the change detector was reactivated
@AndrewSisley
Copy link
Contributor Author

LGTM. Of course I'm biased here so I understand if you want to wait for someone else's approval.

laughing I thought I fixed the change detector earlier but hadn't - so the other have at least until then grin

Was an existing change detector bug - this was just the first test to hit it.

@AndrewSisley AndrewSisley merged commit 84f6c8f into sourcenetwork:develop May 15, 2023
9 checks passed
@AndrewSisley AndrewSisley deleted the 1082-test-restart-with-fred-2 branch May 15, 2023 23:33
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
## Relevant issue(s)

Resolves sourcenetwork#1082 

## Description

Adds DB/Node Restart tests. This will be required by Lens (e.g. to make
sure migrations are not lost on restart). IIRC I think this also found
sourcenetwork#1482 (fixed by Fred).

---------

Co-authored-by: Fred Carle <fredcarle@users.noreply.github.com>
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.

No tests explicitly cover database restart
2 participants