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: Created unit tests to implement native ethers transfer #217

Merged
merged 3 commits into from
May 19, 2024

Conversation

blackbeard002
Copy link
Contributor

closes #206

const askingAmountOrId = [50, 100, 150];

const valueToSend: BigNumber = ethers.utils.parseEther("0.5");

Copy link
Member

Choose a reason for hiding this comment

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

Whitespace can be removed

Copy link
Member

@0xneves 0xneves left a comment

Choose a reason for hiding this comment

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

Requesting minor changes in the name of the tests and whitespace removal. Also requiring a missing test

const valueToSend: BigNumber = ethers.utils.parseEther("0.5");

const expiry = (await blocktimestamp()) + 1000000;

Copy link
Member

Choose a reason for hiding this comment

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

Whitespace can be removed

test/TestSwaplace.test.ts Show resolved Hide resolved
const askingAmountOrId = [50, 100, 150];

const valueToSend: BigNumber = ethers.utils.parseEther("0.5");

Copy link
Member

Choose a reason for hiding this comment

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

Whitespace can be removed

const valueToSend: BigNumber = ethers.utils.parseEther("0.5");

const expiry = (await blocktimestamp()) + 1000000;

Copy link
Member

Choose a reason for hiding this comment

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

Whitespace can be removed

@@ -326,6 +326,44 @@ describe("Swaplace", async function () {
.to.emit(Swaplace, "SwapCreated")
.withArgs(await Swaplace.totalSwaps(), owner.address, zeroAddress);
});

it("Should be able to create a swap with native ethers", async function () {
Copy link
Member

Choose a reason for hiding this comment

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

Use "by the owner" refer, like in line 377

@@ -466,6 +538,69 @@ describe("Swaplace", async function () {
allowed.address,
);
});

it("Should be able to {acceptSwap} with native ethers", async function () {
Copy link
Member

Choose a reason for hiding this comment

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

Add "sent by the owner"

test/TestSwaplace.test.ts Show resolved Hide resolved
@blackbeard002
Copy link
Contributor Author

Hey @0xneves made the requested changes. Ready for review

Copy link
Member

@0xneves 0xneves left a comment

Choose a reason for hiding this comment

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

Requesting minor change:

@@ -339,7 +339,7 @@ describe("Swaplace", async function () {
const askingAmountOrId = [50, 100, 150];

const valueToSend: BigNumber = ethers.utils.parseEther("0.5");

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary whitespace was added here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0xneves sorry, removed it now

Copy link
Member

@0xneves 0xneves left a comment

Choose a reason for hiding this comment

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

Approving these changes

@0xneves 0xneves merged commit 718b4ca into blockful-io:revision May 19, 2024
3 checks passed
@blackbeard002 blackbeard002 deleted the ethTransfer branch May 19, 2024 12:21
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.

None yet

2 participants