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

test: hash consensus: add more frames tests #576

Merged

Conversation

DiRaiks
Copy link
Contributor

@DiRaiks DiRaiks commented Feb 8, 2023

Tests HashConsensus:

  • Add returning fastLaneLengthSlots to getFrameConfig method in hashConsensus contract
  • add more tests for work with frames
  • update access control tests
  • add help methods to HashConsensusTimeTravellable
  • fix code style with prettier
  • add INITIAL_EPOCH const to deployHashConsensus

image

image

@DiRaiks DiRaiks requested a review from skozin February 8, 2023 14:39
@TheDZhon TheDZhon self-requested a review February 8, 2023 19:52
Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution 🚀

test/0.8.9/oracle/hash-consensus-frames.js Outdated Show resolved Hide resolved
test/0.8.9/oracle/hash-consensus-frames.js Outdated Show resolved Hide resolved
@@ -161,7 +236,7 @@ contract('HashConsensus', ([admin, member1, member2]) => {
})

it(`decreasing the frame size cannot decrease the current reference slot`, async () => {
assert.equal(+await consensus.getTime(), computeTimestampAtEpoch(1))
assert.equal(+(await consensus.getTime()), computeTimestampAtEpoch(1))
Copy link
Member

Choose a reason for hiding this comment

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

could we change prettier settings to not add parentheses around await in cases like this one? it's much less readable this way imo

Copy link
Member

Choose a reason for hiding this comment

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

not critical, a nitpick

Copy link
Contributor

Choose a reason for hiding this comment

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

Faced same issues with prettier in different PR. My prettier is pulling config from root of the repo but a lot of other files are not formatted at all. In a different PR, we can update config and format all(or just currently relevant) files.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's do it later in a single batch I suggest

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

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

Magnificient, thanks 👍

it('should revert without manage members and qourum role', async () => {
await assert.reverts(consensus.addMember(member1, 2, { from: account1 }), errorMessage)
it('should revert without MANAGE_MEMBERS_AND_QUORUM_ROLE role', async () => {
await assert.revertsOZAccessControl(
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@TheDZhon TheDZhon merged commit 761a77a into feature/shapella-upgrade Feb 9, 2023
@TheDZhon TheDZhon deleted the feature/shapella-upgrade-tests-tests-frames branch February 9, 2023 08:14
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.

4 participants