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

Update stats and scenario testing #729

Merged
merged 7 commits into from Sep 1, 2019

Conversation

NicolasDP
Copy link
Contributor

@NicolasDP NicolasDP commented Aug 29, 2019

  • missing: update the open API doc

@NicolasDP NicolasDP added A-tests Area: Tests subsys-rest issues or PRs for the REST interface labels Aug 29, 2019
@NicolasDP NicolasDP requested a review from a team August 29, 2019 15:24
@NicolasDP NicolasDP self-assigned this Aug 29, 2019
Copy link
Contributor

@mzabaluev mzabaluev left a comment

Choose a reason for hiding this comment

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

Looks good for the most part.

@@ -133,3 +133,67 @@ pub fn scenario_1(mut context: Context<ChaChaRng>) {

controller.finalize();
}

pub fn scenario_2(mut context: Context<ChaChaRng>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the name be more informative as to what the scenario tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nop. they will be cleaned up when I have a better control of the different scenario and their execution

@@ -55,7 +55,7 @@ fn main() {

introduction(&context);

scenario_1(context.derive());
scenario_2(context.derive());
Copy link
Contributor

Choose a reason for hiding this comment

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

Has scenario_1 fallen into dead code then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

soon

pub fn address(&self, discrimination: Discrimination) -> Address {
self.identifier().to_address(discrimination).into()
}

pub fn increment_counter(&mut self) {
let v : u32 = self.internal_counter.into();
self.internal_counter = account::SpendingCounter::from(v + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

SpendingCounter might benefit from an impl of AddAssign to simplify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree

@NicolasDP NicolasDP force-pushed the update-stats-and-scenario-testing branch from de0c02a to a753df5 Compare August 30, 2019 16:02
@NicolasDP NicolasDP force-pushed the update-stats-and-scenario-testing branch from a753df5 to 8a2e88e Compare September 1, 2019 07:26
@NicolasDP NicolasDP merged commit 858dd00 into master Sep 1, 2019
@NicolasDP NicolasDP deleted the update-stats-and-scenario-testing branch September 1, 2019 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tests Area: Tests subsys-rest issues or PRs for the REST interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants