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(runStake): use test driver rather than data file #5001

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

dtribble
Copy link
Member

@dtribble dtribble commented Apr 3, 2022

test(runStake): use test driver rather than

Description

  • convert the test cases in the data file to use test driver explicitly
  • step converts to a call on the driver.

Testing Considerations

This enables normal developer tool support (operation completion and Intellisense) and stack traces test failure; e.g.,

  Difference:

    {
      brand: Object @Alleged: RUN brand {},
  -   value: 194967256n,
  +   value: 196000000n,
    }

  › approxEqual (packages/run-protocol/test/runStake/test-runStake.js:477:7)
  › Object.checkRUNDebt (packages/run-protocol/test/runStake/test-runStake.js:713:7)
  › async packages/run-protocol/test/runStake/test-runStake.js:942:3

  ─

@dtribble dtribble requested a review from dckc April 3, 2022 23:59
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

Looks consistent with a suggestion you made earlier.

At a glance, I don't see any critical problems. I can take a closer look if we decide this addresses a MN-1 issue.

Commenting out code rather than deleting it is a little odd, but consistent with marking this as DRAFT.

@dckc
Copy link
Member

dckc commented Apr 5, 2022

This seems appropriate to include in #3788

* convert the test cases in the data file to use test driver explicitly
* step converts to a call on the driver.
@dtribble dtribble force-pushed the dt/runstake-tests branch from d3f81e4 to 605776a Compare April 7, 2022 00:39
@dtribble dtribble marked this pull request as ready for review April 7, 2022 00:39
* delete commented-out code
@dtribble dtribble requested a review from dckc April 7, 2022 00:52
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I checked it out and verified that messing with the tests breaks in the expected way.

Good stuff.

@dckc dckc added the automerge:squash Automatically squash merge label Apr 7, 2022
@mergify mergify bot merged commit 0be1f93 into master Apr 7, 2022
@mergify mergify bot deleted the dt/runstake-tests branch April 7, 2022 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants