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

Block constructors to builders conversion in unit tests #3841

Conversation

JerzyStanislawski
Copy link
Contributor

Related to issue: #3817
Done mostly by regex replace.

@dsiganos
Copy link
Contributor

dsiganos commented Jun 17, 2022

Thank you for sending this. We appreciate the effort and the pull request. What was the regex you used?

@dsiganos
Copy link
Contributor

Hi there, the formatting of the code is wrong, please run the script ci/clang-format-do.sh to fix it.
https://github.com/nanocurrency/nano-node/runs/6935306168?check_suite_focus=true

@JerzyStanislawski
Copy link
Contributor Author

JerzyStanislawski commented Jun 17, 2022

Thanks, will fix formatting.
Regarding regex, nothing fancy, depending on block type it was (first expression to find, second one to replace with):

SEND
nano::send_block (?<varname>[a-z0-9_]*) \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<destination>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto ${varname} = builder\n.send()\n.previous(${previous})\n.destination(${destination})\n.balance(${balance})\n.sign(${signprv},${signpub})\n.work(${work})\n.build()

RECEIVE
nano::receive_block (?<varname>[a-z0-9_]*) \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto ${varname} = builder\n.receive()\n.previous(${previous})\n.source(${source})\n.sign(${signprv},${signpub})\n.work(${work})\n.build()

OPEN
nano::open_block (?<varname>[a-z0-9_]*) \((?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto ${varname} = builder\n.open()\n.source(${source})\n.representative(${representative})\n.account(${account})\n.sign(${signprv},${signpub})\n.work(${work})\n.build()

CHANGE
nano::change_block (?<varname>[a-z0-9_]*) \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto ${varname} = builder\n.change()\n.previous(${previous})\n.representative(${representative})\n.sign(${signprv},${signpub})\n.work(${work})\n.build()

STATE
nano::state_block (?<varname>[a-z0-9_]*) \((?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<link>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto ${varname} = builder\n.state()\n.account(${account})\n.previous(${previous})\n.representative(${representative})\n.balance(${balance})\n.link(${link})\n.sign(${signprv},${signpub})\n.work(${work})\n.build()

And for pointers:

SEND
auto (?<varname>[a-z0-9_]*) = std\:\:make_shared\<nano\:\:send_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<destination>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto (?<varname>[a-z0-9_]*) \(std\:\:make_shared\<nano\:\:send_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<destination>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\)\);
auto ${varname} = builder\n.send()\n.previous(${previous})\n.destination(${destination})\n.balance(${balance})\n.sign(${signprv},${signpub})\n.work(${work})\n.build_shared()

RECEIVE
auto (?<varname>[a-z0-9_]*) = std\:\:make_shared\<nano\:\:receive_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto (?<varname>[a-z0-9_]*) \(std\:\:make_shared\<nano\:\:receive_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\)\);
auto ${varname} = builder\n.receive()\n.previous(${previous})\n.source(${source})\n.sign(${signprv},${signpub})\n.work(${work})\n.build_shared()

OPEN
auto (?<varname>[a-z0-9_]*) = std\:\:make_shared\<nano\:\:open_block\> \((?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto (?<varname>[a-z0-9_]*) \(std\:\:make_shared\<nano\:\:open_block\> \((?<source>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\)\);
auto ${varname} = builder\n.open()\n.source(${source})\n.representative(${representative})\n.account(${account})\n.sign(${signprv},${signpub})\n.work(${work})\n.build_shared()

CHANGE
auto (?<varname>[a-z0-9_]*) = std\:\:make_shared\<nano\:\:change_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto (?<varname>[a-z0-9_]*) \(std\:\:make_shared\<nano\:\:change_block\> \((?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\)\);
auto ${varname} = builder\n.change()\n.previous(${previous})\n.representative(${representative})\n.sign(${signprv},${signpub})\n.work(${work})\n.build_shared()

STATE
auto (?<varname>[a-z0-9_]*) = std\:\:make_shared\<nano\:\:state_block\> \((?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<link>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\);
auto (?<varname>[a-z0-9_]*) \(std\:\:make_shared\<nano\:\:state_block\> \((?<account>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<previous>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<representative>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<balance>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<link>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signprv>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*), (?<signpub>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*),(?<work>[a-z0-9\.\:_\s\-\+\*\<\>\(\)\&]*)\)\);
auto ${varname} = builder\n.state()\n.account(${account})\n.previous(${previous})\n.representative(${representative})\n.balance(${balance})\n.link(${link})\n.sign(${signprv},${signpub})\n.work(${work})\n.build_shared()

@dsiganos
Copy link
Contributor

dsiganos commented Jun 28, 2022

This is a huge PR and it is not reasonable to review it in a manual way.
Is it possible to give me the commands you used to auto-generate the code changes?
If I can autogenerate the same output as this PR then I do not need to review the actual changes and I can review the regular expressions only.

@JerzyStanislawski
Copy link
Contributor Author

I didn't use any code generation tools, I just used Replace feature in Visual Studio with regex option and was using it file by file, block type by block type. Then I was manually fixing code related to pointers.
I don't see any reasonable way to automatically check the changes.

@clemahieu
Copy link
Contributor

I was able to do search/replace within VS with these regexs and the diff of the diffs looks equivalent with manual inspection.

@clemahieu clemahieu merged commit 86f1178 into nanocurrency:develop Jul 8, 2022
@JerzyStanislawski JerzyStanislawski deleted the unit_tests_with_block_builders branch July 8, 2022 18:31
gr0vity-dev pushed a commit to gr0vity-dev/nano-node that referenced this pull request Jul 9, 2022
…#3841)

* Block constructors to builders conversion in unit tests

* Formatting fix
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.

3 participants