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

difftest-core (driver): refactor setup to make it more maintainable #658

Merged
merged 66 commits into from
Feb 1, 2023

Conversation

danwt
Copy link
Contributor

@danwt danwt commented Jan 11, 2023

Description

The setup for the diff testing in go was very very horrible. The bad code was isolated to setup.go. This PR majorly improves the maintainability of that file.

Please note: this is by no means a finished job, but I think this PR is a good milestone and now I should evaluate priorities for other tasks.

Recommendation: just merge it. It will be impossible to read the diff. 😓 Tests pass and I really promise this is better! More improvements to come!

This is a pure refactor PR. Not all changes were isolated to setup.go. I moved one test to a new file, and I removed the use of a global variable, which touched core_test.go in a few places.

Thank you

  • Refactor: Changes existing code style, naming, structure, etc.

Daniel added 30 commits January 10, 2023 18:25
commit d1c3ac45289e4e0e0614e4133978f1807401d544
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:15:17 2023 +0000

    Inlines doStuff

commit 5490df90919f11f0d5cc794fa4b7eadc6ba32343
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:14:55 2023 +0000

    Deletes unnecesssary line

commit 8b19fcb42e0f235d129c1b63830c562749142238
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:13:51 2023 +0000

    Extract consumer genesis

commit 4195b858fe5c1cb1c5212a8efbf0cee460cc44a8
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:10:53 2023 +0000

    Extract createConsumerClientGenesisState

commit 08a4d4804926ac029d35937606c17c9a6fe8eb7c
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:07:52 2023 +0000

    Amend: move forgotten line

commit c3637354c649c67bf2fb5fca547a01ca5cd5aea9
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:07:28 2023 +0000

    Extract configureConsumerClientOnProvider

commit 236cf1f0f5eac25607f3354521e474c0bfe98596
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:06:02 2023 +0000

    Move consumer client init on provider

commit 04a438dbddd403d069eb9b6fc260c021c0d752d9
Author: Daniel <[email protected]>
Date:   Wed Jan 11 08:59:11 2023 +0000

    Move configureIBCTestingPath

commit e977309b4205be6078bf9cde2f48f767928723e6
Author: Daniel <[email protected]>
Date:   Wed Jan 11 08:58:44 2023 +0000

    Extract configureIBCTestingPath

commit 4d8d3e83999486dc1c63fa83fc8667dc24c1ca38
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:48:07 2023 +0000

    Moves channel, connections HS to setup body

commit 30d583446e0d70b4ef3d1d5521a60105f0500c35
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:47:03 2023 +0000

    Inlines ibc handshake

commit 0b3d6c4029950e4bc19b871376ae4a0d36d42c87
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:45:13 2023 +0000

    Inlinen create consumer gen

commit 27949cb4f917ac88c0262b95d1c2a85e072e4d42
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:44:12 2023 +0000

    Extract doStuff *fix

commit eed615ecd987497dee9a7b452e9507f7cc97263d
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:43:24 2023 +0000

    Extract doStuff

commit ded978c4270c474c878577267690f04bcc5ab19e
Author: Daniel <[email protected]>
Date:   Tue Jan 10 19:41:48 2023 +0000

    Extracts 1 method
Squashed commit of the following:

commit 21bfdab6d3c98932bf89a36a596a89234c46d83b
Author: Daniel <[email protected]>
Date:   Wed Jan 11 11:31:13 2023 +0000

    Gets rid of sendEmpty

commit c7564905073427329114e665d4b6f270e7055960
Author: Daniel <[email protected]>
Date:   Wed Jan 11 09:57:31 2023 +0000

    Adds debug session
Copy link
Contributor

@sainoe sainoe left a comment

Choose a reason for hiding this comment

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

approved based on the recommendations

@mpoke mpoke requested a review from MSalopek January 27, 2023 15:34
@shaspitz
Copy link
Contributor

@sainoe @MSalopek are we able to merge this now? No significant diff test regressions I assume?

@sainoe
Copy link
Contributor

sainoe commented Feb 1, 2023

Yep. Note that I had to revert the changes using cryptoidentity after merging with main. The diff testing still creates validators' consensus and operator addresses using a single pubkey. I will address that in a separate PR.

@sainoe sainoe merged commit b89c46e into main Feb 1, 2023
@sainoe sainoe deleted the danwt/difftest-core-setup-refactor branch February 1, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants