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

end2endtest refactoring #874

Merged
merged 2 commits into from
Apr 24, 2023
Merged

end2endtest refactoring #874

merged 2 commits into from
Apr 24, 2023

Conversation

altergui
Copy link
Contributor

@altergui altergui commented Apr 4, 2023

  • move mkTreeVoteTest out of main.go into its own file, rename to something less cryptic (plaintextElectionTest), operation plaintextelection instead of vtest

@altergui altergui self-assigned this Apr 4, 2023
@altergui altergui force-pushed the feat/end2endtest_refactor branch 2 times, most recently from 56d9f19 to d879b33 Compare April 5, 2023 11:32
@altergui altergui marked this pull request as ready for review April 5, 2023 15:37
@altergui
Copy link
Contributor Author

altergui commented Apr 5, 2023 •
edited
Loading

i want to make a couple of small improvements yet:

  • running some tests concurrently so integration test doesn't take 12m 😵
  • cleaning git history

but i consider this ready for review already, feedback welcome

@altergui altergui changed the title (WIP) end2endtest refactoring end2endtest refactoring Apr 5, 2023
@altergui
Copy link
Contributor Author

altergui commented Apr 5, 2023

notable things done until now in this PR:

p4u
p4u previously requested changes Apr 6, 2023
View reviewed changes
Copy link
Member

@p4u p4u left a comment

Choose a reason for hiding this comment

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

Great job @altergui, the code looks quite neat.

I'd suggest some changes on the general structure.

  1. Add the test implementations into one ore several subdirectories and packages, they might be group by type, i.e "voting_tests" "account_tests"

  2. In the main package directory, you might have the interface definition and the cmd.

  3. You might want to have a "shared" directory package, with the functions that are shared among tests. Be aware of avoiding cyclic imports.

apiclient/helpers.go Outdated Show resolved Hide resolved
apiclient/vote.go Outdated Show resolved Hide resolved
apiclient/vote.go Outdated Show resolved Hide resolved
cmd/end2endtest/account.go Outdated Show resolved Hide resolved
cmd/end2endtest/account.go Outdated Show resolved Hide resolved
cmd/end2endtest/encrypted.go Outdated Show resolved Hide resolved
cmd/end2endtest/encrypted.go Outdated Show resolved Hide resolved
cmd/end2endtest/main.go Outdated Show resolved Hide resolved
cmd/end2endtest/main.go Show resolved Hide resolved
cmd/end2endtest/plaintext.go Show resolved Hide resolved
mariajdab
mariajdab reviewed Apr 7, 2023
View reviewed changes
cmd/end2endtest/encrypted.go Outdated Show resolved Hide resolved
cmd/end2endtest/encrypted.go Show resolved Hide resolved
@altergui altergui force-pushed the feat/end2endtest_refactor branch 4 times, most recently from 645325a to e37b2c7 Compare April 17, 2023 14:41
@altergui altergui force-pushed the feat/end2endtest_refactor branch 5 times, most recently from be71bf3 to 67a2622 Compare April 18, 2023 17:48
@altergui
Copy link
Contributor Author

a most notable change that i want to highlight is that now job_compose_test runs the tests concurrently, which led to uncovering a couple of bugs like #890 that i either worked around or fixed. previously we simply said "ok let's not do this concurrently".

so the run time is halved to 4mins instead of the usual 8-12 minutes, which is simply GREAT. it takes roughly the same as job_go_test, which is also run concurrently. so now the whole CI takes 4 mins, instead of go_test taking 4 mins and compose_test taking ages

@altergui altergui requested review from p4u and mariajdab April 18, 2023 17:57
@altergui
Copy link
Contributor Author

altergui commented Apr 18, 2023 •
edited
Loading

for me this is ready to merge as-is: even though it could be improved (returns instead of Fatals, deduplicating code), it doesn't break anything and already brings a lot of value, so i believe it's better to merge it (also unblocking #873) and open a second iteration of refactoring in a new PR

@altergui altergui mentioned this pull request Apr 18, 2023
Closed
8 tasks
@altergui altergui force-pushed the feat/end2endtest_refactor branch 2 times, most recently from 92c9a5a to d50e7df Compare April 19, 2023 12:16
@mariajdab
Copy link
Contributor

Just one conflict to resolve I think

altergui and others added 2 commits April 24, 2023 09:08
@altergui
e2etest: general refactor
25284d8 
e2etest: refactor operations structure (VochainTest interface)

* implement VochainTest interface to separate Setup, Run and Teardown
* populate ops var (now a map[string]operation) in init() of each file
* rename operations
 * anonvoting -> anonelection
 * tokentransactions -> tokentxs
 * vtest -> plaintextelection
* new test encryptedelection

* apiclient: add SecretUntilTheEnd support to apiclient.Vote()
  * New methods WaitUntilElectionKeys, ElectionKeys,
    prepareVoteEnvelope, prepareVotePackageBytes

* ci: update tests_to_run
 * add e2etest_plaintextelection
 * add e2etest_encryptedelection
 * run new end2endtests concurrently
 * drop legacy vochaintests

* fix: tokentxs WaitUntilNextBlock before checking resulting state
@mariajdab @altergui
e2etest: more refactoring
4d3ef33 
* add helper methods
* helpers remove unnecessary code
* main, add election base struct and remove code that already exists in helpers
* update getFaucetPackage calls
* use helper methods in plaintext test
* remove unnecessary code
* update generateProofs
* update plaintextelection
* update generateProofs for correctly support anonymous voting proofs
* port anonelection to use helpers
* port encryptedelection to use helpers
* cleaning
* main sort imports
* wait longer in some places
* improve approach in Setup with a flexible setupElection
@altergui altergui force-pushed the feat/end2endtest_refactor branch 2 times, most recently from 05d0b26 to 4d3ef33 Compare April 24, 2023 16:06
@coveralls
Copy link

Coverage Status

Coverage: 50.887% (-3.06%) from 53.946% when pulling 4d3ef33 on feat/end2endtest_refactor into 1c70b57 on master.

@altergui altergui dismissed p4u’s stale review April 24, 2023 16:17

out of band agreement

@altergui altergui merged commit 681e990 into master Apr 24, 2023
@altergui altergui deleted the feat/end2endtest_refactor branch April 24, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariajdab mariajdab mariajdab approved these changes

@lucasmenendez lucasmenendez Awaiting requested review from lucasmenendez

@p4u p4u Awaiting requested review from p4u

Assignees

@altergui altergui

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@altergui @mariajdab @coveralls @p4u