-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Return block in callbackified blockchain #986
Conversation
@@ -1,2 +1 @@ | |||
export type Callback<T = void> = ((error: NonNullable<any>) => void) & | |||
((error: null | undefined, value: T) => void); | |||
export type Callback<T = void> = (error: any, value: T) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the type change that triggers all the other typescript changes. This makes sense to me, but I have no idea what I'm doing.
const genesis = createBlock(0); | ||
const one = createBlock(1); | ||
await promisify<Block>(callbackifiedBlockchain.putBlock)(genesis); | ||
await promisify<Block>(callbackifiedBlockchain.putBlock)(one); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ugly, I'd rather have a promisifiedCallbackifiedBlockchain
(without that name, I swear) and just duplicate the previous test as is.
@@ -146,6 +176,34 @@ describe("HardhatBlockchain", () => { | |||
assert.equal(await blockchain.getBlock(blockThree.hash()), undefined); | |||
}); | |||
|
|||
it.skip("removes the block and all subsequent ones (callbackified)", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is a duplicated version of the previous one, like the other added tests. But this one fails because of this line:
I don't know if that line is fine, and so this tests needs to be different, or if that undefined check should be removed
This fix is literally one line, but I also modified a bunch of typescript stuff to make typescript detect the previous issue. I'm not sure at all about those changes.
I also added some minimal tests for the callbackified version of the
Blockchain
class. One of them is skipped for reasons I will explain in the comments.