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.

Sign up for GitHub

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

Jump to bottom

Manual test for syncProgress #977

Merged
merged 2 commits into from
Nov 7, 2019
Merged

Manual test for syncProgress #977

merged 2 commits into from
Nov 7, 2019

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Nov 7, 2019

Issue Number

#921

Overview

  • I have added a manual test making sure 1. State is Ready, 2. syncProgress drops when node is killed, 3. syncProgress reaches Ready again when node is re-started.

Comments

@Anviking Anviking self-assigned this Nov 7, 2019
@Anviking Anviking mentioned this pull request Nov 7, 2019
Closed
piotr-iohk
piotr-iohk approved these changes Nov 7, 2019
View reviewed changes
Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

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

lgtm!

KtorZ
KtorZ approved these changes Nov 7, 2019
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

cc @piotr-iohk

@KtorZ
Copy link
Member

KtorZ commented Nov 7, 2019

bors r+

@KtorZ
Copy link
Member

KtorZ commented Nov 7, 2019

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 7, 2019

Canceled

@KtorZ
Copy link
Member

KtorZ commented Nov 7, 2019

@Anviking I just noticed the file location.
May we put this in cardano-wallet-jormungandr 🙏 ?

@piotr-iohk
Copy link
Contributor

May we put this in cardano-wallet-jormungandr...

Actually, I think it is more convenient to have all manual procedures in the same location, so one doesn't have to switch dirs too much when going from one to another... we don't have a lot of them but still.. What do you think @KtorZ, @Anviking ?

@Anviking
Copy link
Member Author

Anviking commented Nov 7, 2019 •
edited
Loading

have all manual procedures in the same location

Maybe. There is already an existing lib/jormungandr/test/manual/TrackBlockHeight.md however.

I just renamed the addition to lib/jormungandr/test/manual/SyncProgress.md to match it.

What would feel off to me is that this test is really testing functionality in core. But it does that using using jormungandr. 🤔

@piotr-iohk
Copy link
Contributor

@Anviking that makes sense. Maybe it'd be better actually to have an additional file with an index/table of contents with links to all manual procedures for convenience.
Something I can think of and add later, no worries.

@KtorZ
Copy link
Member

KtorZ commented Nov 7, 2019

Actually, I think it is more convenient to have all manual procedures in the same location

I agree. Then, it makes sense to have them all in a parent location, at the root of the project perhaps? Let's move them all in a test/manual folder located in the root.

@Anviking Anviking requested a review from KtorZ November 7, 2019 14:24
@Anviking
Copy link
Member Author

Anviking commented Nov 7, 2019

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 7, 2019
@iohk-bors @Anviking
Merge #977
fb932d3 
977: Manual test for syncProgress r=Anviking a=Anviking

# Issue Number

#921 

# Overview

- [x] I have added a manual test making sure 1. State is `Ready`, 2. syncProgress drops when node is killed, 3. syncProgress reaches `Ready` again when node is re-started.

# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 7, 2019

Build succeeded

@iohk-bors iohk-bors bot merged commit 894a48c into master Nov 7, 2019
@Anviking Anviking deleted the anviking/921/manual-test branch November 7, 2019 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piotr-iohk piotr-iohk piotr-iohk approved these changes

@KtorZ KtorZ Awaiting requested review from KtorZ

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Anviking @KtorZ @piotr-iohk