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

Rupan/reward actor tests #403

Merged
merged 8 commits into from
May 11, 2020
Merged

Conversation

RajarupanSampanthan
Copy link
Contributor

@RajarupanSampanthan RajarupanSampanthan commented May 9, 2020

Summary of changes
Changes introduced in this pull request:

  • Ported reward actor test and fixed some minor bugs found

Reference issue to close (if applicable)

Closes #315

Other information and links

Didn't get to do this but we should probably create a func as a wrapper around call to revert state if call is being used instead of manually fixing state every time.

@RajarupanSampanthan
Copy link
Contributor Author

Not sure why test is failing because cargo test passes on my laptop

@austinabell
Copy link
Contributor

Not sure why test is failing because cargo test passes on my laptop

Yeah don't worry that's not anything to do with this PR, @ec2 had that on his branch. Just the linker is not working consistently on the CI rn

Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

I can't comment on it since it's outside of changes, but can you add #[allow(dead_code)] on the get_state() function in the mock rt? This new test does not use it and it adds a warning to the test execution

vm/actor/tests/common/mod.rs Show resolved Hide resolved
vm/actor/tests/reward_actor_test.rs Outdated Show resolved Hide resolved
vm/actor/tests/reward_actor_test.rs Outdated Show resolved Hide resolved
vm/actor/tests/reward_actor_test.rs Outdated Show resolved Hide resolved
vm/actor/tests/reward_actor_test.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

ah yes, I see now how weird their test setup is, would be nice to be able to share that expect fail logic across tests but we can do that later/ doesn't really matter

vm/actor/tests/common/mod.rs Outdated Show resolved Hide resolved
ticket_count: 0,
};

//Expect call to fail because actor doesnt have enough tokens to reward
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Expect call to fail because actor doesnt have enough tokens to reward
// Expect call to fail because actor doesnt have enough tokens to reward

@RajarupanSampanthan RajarupanSampanthan merged commit 81257df into master May 11, 2020
@RajarupanSampanthan RajarupanSampanthan deleted the rupan/reward_actor_tests branch May 11, 2020 17:45
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.

Port over tests for Reward Actor
3 participants