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

Add NRVO tests and make SCP types serializable with the serializer #687

Draft
wants to merge 1 commit into
base: v0.x.x
Choose a base branch
from

Conversation

Geod24
Copy link
Collaborator

@Geod24 Geod24 commented Mar 20, 2020

This is a requirement for switching away from Vibe.d's REST module.

This is still a WIP, I want to add some sensible data in SCPStatement.
I also have an idea to systematically test NRVO on struct, I just need a bit of time.

@Geod24 Geod24 added the type-enhancement An improvement of existing functionalities label Mar 20, 2020
@codecov
Copy link

codecov bot commented Mar 20, 2020

Codecov Report

Merging #687 (cbcd60b) into v0.x.x (f948c0d) will decrease coverage by 37.32%.
The diff coverage is 53.33%.

❗ Current head cbcd60b differs from pull request most recent head dbefcd1. Consider uploading reports for the commit dbefcd1 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           v0.x.x     #687       +/-   ##
===========================================
- Coverage   88.53%   51.20%   -37.33%     
===========================================
  Files         146       39      -107     
  Lines       13989     2113    -11876     
===========================================
- Hits        12385     1082    -11303     
+ Misses       1604     1031      -573     
Flag Coverage Δ
integration 51.20% <53.33%> (+36.60%) ⬆️
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
source/scpd/Cpp.d 65.11% <0.00%> (-27.81%) ⬇️
source/scpd/types/Stellar_SCP.d 82.85% <0.00%> (+28.31%) ⬆️
source/agora/common/Serializer.d 71.95% <100.00%> (ø)
source/agora/consensus/data/Enrollment.d 0.00% <0.00%> (-100.00%) ⬇️
source/agora/utils/Test.d 0.00% <0.00%> (-95.00%) ⬇️
source/agora/utils/SCPPrettyPrinter.d 0.00% <0.00%> (-86.24%) ⬇️
source/agora/utils/PrettyPrinter.d 8.00% <0.00%> (-81.86%) ⬇️
source/agora/consensus/EnrollmentManager.d 16.66% <0.00%> (-80.11%) ⬇️
source/agora/consensus/protocol/Nominator.d 56.16% <0.00%> (-35.49%) ⬇️
source/agora/common/Amount.d 64.70% <0.00%> (-32.38%) ⬇️
... and 155 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 109bf3a...dbefcd1. Read the comment docs.

@AndrejMitrovic
Copy link
Contributor

This is still a WIP, I want to add some sensible data in SCPStatement.

The prettify PR I made has some tests you can borrow from.

/// Check that `self is &this`
public void check (string message, int line = __LINE__) const
{
if (ptr(this.self) !is ptr(this))
Copy link
Contributor

Choose a reason for hiding this comment

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

would this.self !is ptr(this) not work?

@Geod24
Copy link
Collaborator Author

Geod24 commented Mar 23, 2020

Fixes #691

@Geod24
Copy link
Collaborator Author

Geod24 commented Apr 6, 2020

I have update this to the HEAD of my current WIP, if anyone's interested in looking at where it's heading towards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement An improvement of existing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure NRVO is done through the Deserializer to avoid memory corruption
2 participants