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

Factor test data out of test module #1

Closed
wants to merge 364 commits into from
Closed

Factor test data out of test module #1

wants to merge 364 commits into from

Conversation

zancas
Copy link

@zancas zancas commented Nov 18, 2018

I renamed tests.rs to tests/mod.rs for reasons I am not entirely clear on this made it easier to use mod namespaces to organize test data.

Organizing test data was my primary motivation for this work. I wanted to be able to easily read the test code in the first-tests-later-mod.rs file. This solution worked for me.

While I was hacking this out @str4d (who was unaware I was poking about) updated the test data.. I add his update with a commit message pointing to the original commit.

@str4d
Copy link
Owner

str4d commented Nov 20, 2018

Hey, this is a great refactor, thanks! It can't be merged as-is due to a conflict, and also because the TestVector structs need to be semantically separated like before. But I'll cherry-pick this onto my branch and tweak it appropriately 🙂

@zancas
Copy link
Author

zancas commented Nov 20, 2018

Great! Thanks! Will that be here:

https://github.com/str4d/librustzcash/tree/zcash-transaction-primitives

?

I have two other things to commit, and I'd like to PR them against the most convenient branch.

@zancas
Copy link
Author

zancas commented Nov 21, 2018

Ahh.. you mean you prefer each fn test_.. to define its own TestVector?

@zancas
Copy link
Author

zancas commented Nov 23, 2018

@str4d , I will refactor again on top of your latest, with the TestVector structs (plural) held distinct with each test. So that the code can be cleanly merged.

@zancas
Copy link
Author

zancas commented Nov 27, 2018

When you say the TestVector struct needs to be semantically separated, do you mean you want a separate struct for each test?

@zancas
Copy link
Author

zancas commented Nov 27, 2018

OK @str4d I refactored atop your latest, to remove the need for a cherry-pick.

I'm not exactly clear on whether the TestVector struct is defined optimally.

@zancas zancas mentioned this pull request Nov 28, 2018
str4d pushed a commit that referenced this pull request Jan 6, 2019
Update dependencies and traits
str4d pushed a commit that referenced this pull request Jan 6, 2019
Migrate curve traits and tests, and WNAF, from pairing
str4d and others added 21 commits April 5, 2019 21:05
This crate exposes both the ChaCha20Poly1305 IETF construction, and the
underlying ChaCha20 IETF primitive, removing the need for depending on
our own fork of the previous chacha20-poly1305-aead crate.
The crypto_api_chachapoly uses two new features introduced in 1.32:

- Self struct constructors
- u64::to_le_bytes()
Use wrapping function to directly disable integer overflow protection.
Place bellman multicore operations behind a (default) feature flag
Closes zcash#52. Fix test error "attempt to shift right with overflow".
str4d added 19 commits October 8, 2019 17:43
This enables basic verification of chain validity when CompactBlocks are
received without the full header.
Checking for spent notes in a block is still not completely constant
time, due to filtering out negative results of the constant-time
comparison.

Part of zcash#84.
cargo fmt does not build the code, and running it in a fresh clone of
the codebase will fail because the protobuf code has not been generated.
cargo fmt does not build the code, and running it in a fresh clone of
the codebase will fail because the protobuf code has not been generated.
CompactBlock parsing and scanning
@str4d
Copy link
Owner

str4d commented Oct 14, 2019

The branch this PR is targeting was merged upstream in zcash#39, so this PR should probably be closed here and rebased / recreated on upstream master.

@zancas
Copy link
Author

zancas commented Oct 15, 2019

I'll put this on the TODO list.

@zancas
Copy link
Author

zancas commented Oct 16, 2019

I notice automated tests run when I update the PR against the @str4d fork. I'll let them run, before possibly closing and re-opening against origin.

@zancas
Copy link
Author

zancas commented Oct 16, 2019

Alright, all checks passed.... closing an reopening against origin/master.

@zancas zancas closed this Oct 16, 2019
str4d pushed a commit that referenced this pull request Mar 4, 2020
str4d pushed a commit that referenced this pull request Jun 2, 2021
ZIP-225/244 #1: Minor refactoring and preparatory updates.
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.