fix: make gossip portal-interop test auto generate comments for errors #118
Conversation
KolbyML
force-pushed
the
make-errors-auto-gen
branch
2 times, most recently
from
51ae840 to
3e76fa4
Compare
KolbyML
changed the title
| // wait content_vec.len() seconds for data to propagate, giving more time if more items are propagating | ||
| tokio::time::sleep(Duration::from_secs(test_data.len() as u64)).await; | ||
|
|
||
| let (first_header_key, first_header_value) = test_data.get(0).unwrap(); |
There was a problem hiding this comment.
Could you elaborate on what is happening here, what the first header and last header are, why we set the first as last, and how this relates to comments?
There was a problem hiding this comment.
Last header seen. We are setting it here because the rust borrow checker won't allow us to set unassigned variables.
So at the start the last seen header would be the first header, because it is the only one we know about at the start therefore being the last header seen
There was a problem hiding this comment.
It's worth adding a docstring to the code for reference
KolbyML
force-pushed
the
make-errors-auto-gen
branch
from
3e76fa4 to
5c53860
Compare
KolbyML
force-pushed
the
make-errors-auto-gen
branch
from
5c53860 to
e14684a
Compare
| "7000000 receipt", "15600000 (post-merge) header", "15600000 (post-merge) block body", "15600000 (post-merge) receipt", | ||
| "17510000 (post-shanghai) header", "17510000 (post-shanghai) block body", "17510000 (post-shanghai) receipt"]; | ||
|
|
||
| // wait content_vec.len() seconds for data to propagate, giving more time if more items are propagating |
There was a problem hiding this comment.
// wait test_data.len() seconds ...
There was a problem hiding this comment.
I am not sure what you are trying to imply?
There was a problem hiding this comment.
If I'm not mistaken, the comment is incorrect. We are using test_data.len() in the line below to figure out how long the wait should be, so the comment should be updated with test_data.len() instead of content_vec.len()
There was a problem hiding this comment.
Oh I see what you mean, must have forgot to to copy the commit when I took copy pasted that timeout from trin-bridge tests I will fix that.
| // wait content_vec.len() seconds for data to propagate, giving more time if more items are propagating | ||
| tokio::time::sleep(Duration::from_secs(test_data.len() as u64)).await; | ||
|
|
||
| let (first_header_key, first_header_value) = test_data.get(0).unwrap(); |
There was a problem hiding this comment.
It's worth adding a docstring to the code for reference
| if let HistoryContentKey::BlockHeaderWithProof(_) = &content_key { | ||
| last_header_seen = (content_key.clone(), content_value.clone()); | ||
| } | ||
| let comment = |
There was a problem hiding this comment.
nitpick something likecontent_details might be a more useful var name here
| match client_b.rpc.local_content(content_key.clone()).await { | ||
| Ok(possible_content) => { | ||
| match possible_content { | ||
| match possible_content { | ||
| PossibleHistoryContentValue::ContentPresent(content) => { | ||
| if content != content_value { |
There was a problem hiding this comment.
I know you didn't write this code, but could you update these var names to expected_key/expected_value and actual_key/actual_value. When doing comparisons on tested data, this kind of naming scheme makes it much simpler to figure out what's actually being tested, which var is the test data, etc.
| content_type | ||
| ) | ||
| } else { | ||
| unreachable!("History test dated is formatted incorrectly") |
There was a problem hiding this comment.
Maybe just expand in this comment as to what most likely went wrong and caused the invalid formatting
KolbyML
force-pushed
the
make-errors-auto-gen
branch
from
e77e6b8 to
c59bf1c
Compare
|
@njgheorghita I attempted to resolve the concerns, I was a little confused. So let me know what you think and hopefully we can get this in! |
Since we can dynamically add tests over time we also need a way of generating the error comments dynamically