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: reduce flaky tests #1266

Merged
merged 8 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,6 @@ yarn run lint

### Test

Disable API tests OR define all required explorer API keys.

```bash
export DISABLE_API_TESTS=1
# OR
export EXPLORER_API_KEY_MAINNET=
export EXPLORER_API_KEY_RINKEBY=
export EXPLORER_API_KEY_FUSE=
export EXPLORER_API_KEY_MATIC=
export EXPLORER_API_KEY_FANTOM=
```

Test all the packages in the monorepo.

```bash
Expand Down
40 changes: 17 additions & 23 deletions packages/integration-test/test/node-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ const wallet = Wallet.fromMnemonic(mnemonic).connect(provider);
// eslint-disable-next-line no-magic-numbers
jest.setTimeout(30000);

const requestCreationHashBTC: Types.IRequestInfo = {
currency: 'BTC',
expectedAmount: '1000',
payee: payeeIdentity,
payer: payerIdentity,
};

const requestCreationHashUSD: Types.IRequestInfo = {
currency: 'USD',
expectedAmount: '1000',
Expand Down Expand Up @@ -89,16 +82,16 @@ const waitForConfirmation = async (input: Request | Types.IRequestDataWithEvents
}, 5000);

// Return the confirmation result.
const promiseResults = await Promise.all([waitForConfirmationPromise, mineBlockPromise]);
const [requestData] = await Promise.all([waitForConfirmationPromise, mineBlockPromise]);
clearTimeout(timeout);
return promiseResults[0];
return requestData;
};

describe('Request client using a request node', () => {
it('can create a request, change the amount and get data', async () => {
// Create a request
const request = await requestNetwork.createRequest({
requestInfo: requestCreationHashBTC,
requestInfo: requestCreationHashUSD,
signer: payeeIdentity,
});
expect(request).toBeInstanceOf(Request);
Expand Down Expand Up @@ -194,7 +187,7 @@ describe('Request client using a request node', () => {
it('can create requests and get them fromIdentity and with time boundaries', async () => {
// create request 1
const requestCreationHash1: Types.IRequestInfo = {
currency: 'BTC',
currency: 'USD',
expectedAmount: '100000000',
payee: payeeIdentity,
payer: payerIdentity,
Expand All @@ -219,7 +212,7 @@ describe('Request client using a request node', () => {

// create request 2
const requestCreationHash2: Types.IRequestInfo = {
currency: 'BTC',
currency: 'USD',
expectedAmount: '1000',
payee: payeeIdentity,
payer: payerIdentity,
Expand Down Expand Up @@ -282,7 +275,7 @@ describe('Request client using a request node', () => {
];
const request1: Request = await requestNetwork.createRequest({
requestInfo: {
currency: 'BTC',
currency: 'USD',
expectedAmount: '100000000',
payee: payeeIdentity,
payer: payerSmartContract,
Expand All @@ -295,7 +288,7 @@ describe('Request client using a request node', () => {
// create request 2 to be sure it is not found when search with smart contract identity
await requestNetwork.createRequest({
requestInfo: {
currency: 'BTC',
currency: 'USD',
expectedAmount: '1000',
payee: payeeIdentity,
payer: payerIdentity,
Expand Down Expand Up @@ -330,7 +323,7 @@ describe('Request client using a request node', () => {
const request = await requestNetwork._createEncryptedRequest(
{
requestInfo: {
...requestCreationHashBTC,
...requestCreationHashUSD,
...{ timestamp },
},
signer: payeeIdentity,
Expand Down Expand Up @@ -377,7 +370,7 @@ describe('Request client using a request node', () => {
const request = await requestNetwork._createEncryptedRequest(
{
requestInfo: {
...requestCreationHashBTC,
...requestCreationHashUSD,
...{ timestamp },
},
signer: payeeIdentity,
Expand All @@ -398,7 +391,8 @@ describe('Request client using a request node', () => {
expect(requestData.pending!.state).toBe(Types.RequestLogic.STATE.CREATED);
expect(requestData.meta!.transactionManagerMeta.encryptionMethod).toBe('ecies-aes256-gcm');

await waitForConfirmation(request);
const confirmedRequest = await waitForConfirmation(request);
expect(confirmedRequest.state).toBe(Types.RequestLogic.STATE.CREATED);

// Fetch the created request by its id
const fetchedRequest = await requestNetwork.fromRequestId(request.requestId);
Expand All @@ -425,18 +419,18 @@ describe('Request client using a request node', () => {
expect(fetchedRequestData.state).toBe(Types.RequestLogic.STATE.ACCEPTED);

const increaseData = await request.increaseExpectedAmountRequest(
requestCreationHashBTC.expectedAmount,
requestCreationHashUSD.expectedAmount,
payerIdentity,
);
await waitForConfirmation(increaseData);

await fetchedRequest.refresh();
expect(fetchedRequest.getData().expectedAmount).toEqual(
String(Number(requestCreationHashBTC.expectedAmount) * 2),
String(Number(requestCreationHashUSD.expectedAmount) * 2),
);

const reduceData = await request.reduceExpectedAmountRequest(
Number(requestCreationHashBTC.expectedAmount) * 2,
Number(requestCreationHashUSD.expectedAmount) * 2,
payeeIdentity,
);
await waitForConfirmation(reduceData);
Expand All @@ -458,7 +452,7 @@ describe('Request client using a request node', () => {
const encryptedRequest = await requestNetwork._createEncryptedRequest(
{
requestInfo: {
...requestCreationHashBTC,
...requestCreationHashUSD,
...{ timestamp },
},
signer: payeeIdentity,
Expand All @@ -470,7 +464,7 @@ describe('Request client using a request node', () => {
// Create a plain request
const plainRequest = await requestNetwork.createRequest({
requestInfo: {
...requestCreationHashBTC,
...requestCreationHashUSD,
...{ timestamp },
},
signer: payeeIdentity,
Expand Down Expand Up @@ -512,7 +506,7 @@ describe('Request client using a request node', () => {
const request = await requestNetwork._createEncryptedRequest(
{
requestInfo: {
...requestCreationHashBTC,
...requestCreationHashUSD,
...{ timestamp },
},
signer: payeeIdentity,
Expand Down
12 changes: 0 additions & 12 deletions packages/payment-detection/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,6 @@ yarn codegen

# Test

The ETH `InfoRetriever` tests require explorer API keys. Before running the
payment-detection tests, define `DISABLE_API_TESTS` or define all required
explorer API keys.

```bash
export DISABLE_API_TESTS=1
# OR
export EXPLORER_API_KEY_MAINNET=
export EXPLORER_API_KEY_RINKEBY=
export EXPLORER_API_KEY_FUSE=
export EXPLORER_API_KEY_MATIC=
export EXPLORER_API_KEY_FANTOM=

yarn run test
```
45 changes: 45 additions & 0 deletions packages/payment-detection/test/eth/etherscan-fixtures.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { BigNumber } from 'ethers';

export default [
// etherscan
{
hash: '0x0b53c5296a7b286fef52336529f3934584fea116725d1fe4c59552e926229059',
type: 0,
accessList: null,
blockHash: '0x8535994e0a366ccfc61f5f925ce59404346569342bda1d80ccedfa6fed24c21a',
blockNumber: 9082338,
transactionIndex: 151,
confirmations: 9581599,
from: '0xc12F17Da12cd01a9CDBB216949BA0b41A6Ffc4EB',
gasPrice: BigNumber.from('0x02540be400'),
gasLimit: BigNumber.from('0x5288'),
to: '0xc12F17Da12cd01a9CDBB216949BA0b41A6Ffc4EB',
value: BigNumber.from('0x21'),
nonce: 18,
data: '0x9649a1a4dd5854ed',
creates: null,
chainId: 0,
timestamp: 1575969371,
},

// polygonscan
{
hash: '0x50af07756eb07bb0eb29943cb7206d8359c829aef7f6dad50f61b488c2790c1c',
type: 0,
accessList: null,
blockHash: '0x186a3f7a4a49ef302223bc72cd45a07f01a7c0c6bf1fc3127fc97de7879874f5',
blockNumber: 17182533,
transactionIndex: 108,
confirmations: 33274506,
from: '0x4E64C2d06d19D13061e62E291b2C4e9fe5679b93',
gasPrice: BigNumber.from('0x02f4d4f484'),
gasLimit: BigNumber.from('0x5288'),
to: '0x4E64C2d06d19D13061e62E291b2C4e9fe5679b93',
value: BigNumber.from('0x038d7ea4c68000'),
nonce: 11,
data: '0x5f05d421a5ed3558',
creates: null,
chainId: 0,
timestamp: 1627050587,
},
];
116 changes: 49 additions & 67 deletions packages/payment-detection/test/eth/info-retriever.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { PaymentTypes } from '@requestnetwork/types';
import { EthInputDataInfoRetriever } from '../../src/eth/info-retriever';
import PaymentReferenceCalculator from '../../src/payment-reference-calculator';
import etherscanFixtures from './etherscan-fixtures';
import { providers } from 'ethers';

describe('api/eth/info-retriever', () => {
beforeAll(() => {
jest
.spyOn(providers.EtherscanProvider.prototype, 'getHistory')
.mockResolvedValue(etherscanFixtures as any);
});

// In this test, we're looking this transaction:
// https://etherscan.io/tx/0x0de1759d8b246939e370e1d0509e3ed6f1d5f4f5b79735636c0283b64ff6f5ed
it('can get the balance of an address', async () => {
Expand All @@ -11,7 +19,8 @@ describe('api/eth/info-retriever', () => {
'01000',
'1234567890123456',
paymentAddress,
); // 9649a1a4dd5854ed
);
expect(paymentReference).toBe('9649a1a4dd5854ed');

const infoRetriever = new EthInputDataInfoRetriever(
paymentAddress,
Expand All @@ -20,20 +29,19 @@ describe('api/eth/info-retriever', () => {
paymentReference,
);
const events = await infoRetriever.getTransferEvents();

// If this assertion fails, another transaction with the data `9649a1a4dd5854ed`
// has been set to the address `0xc12F17Da12cd01a9CDBB216949BA0b41A6Ffc4EB`
expect(events).toHaveLength(1);

expect(events[0].name).toBe('payment');
expect(events[0].amount).toBe('33');
expect(typeof events[0].timestamp).toBe('number');
expect(events[0].parameters!.txHash).toBe(
'0x0b53c5296a7b286fef52336529f3934584fea116725d1fe4c59552e926229059',
);
expect(typeof events[0].parameters!.block).toBe('number');
expect(typeof events[0].parameters!.confirmations).toBe('number');
}, 10000);
expect(events).toMatchObject([
{
amount: '33',
name: 'payment',
parameters: {
block: 9082338,
confirmations: 9581599,
txHash: '0x0b53c5296a7b286fef52336529f3934584fea116725d1fe4c59552e926229059',
},
timestamp: 1575969371,
},
]);
});

it('throws when trying to use it in local', async () => {
const infoRetreiver = new EthInputDataInfoRetriever(
Expand All @@ -45,60 +53,34 @@ describe('api/eth/info-retriever', () => {
await expect(infoRetreiver.getTransferEvents()).rejects.toThrowError();
});

// Utility for conditionally skipping tests
const describeIf = (
condition: any,
...args: [string | number | Function | jest.FunctionLike, jest.EmptyFunction]
) => (condition ? describe(...args) : describe.skip(...args));
it('can detect a MATIC payment to self', async () => {
// NB: The from and to are the same
Copy link
Member

Choose a reason for hiding this comment

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

It might just be me but I always get tripped up by the NB acronym for Nota Bene. I think "Note" would be more clear.

Suggested change
// NB: The from and to are the same
// Note: The from and to are the same

const paymentAddress = '0x4E64C2d06d19D13061e62E291b2C4e9fe5679b93';
const paymentReference = PaymentReferenceCalculator.calculate(
'01b809015dcda94dccfc626609b0a1d8f8e656ec787cf7f59d59d242dc9f1db0ca',
'a1a2a3a4a5a6a7a8',
paymentAddress,
);

// Skip tests if build is from external fork or API tests are disabled
// External forks cannot access secret API keys
describeIf(!process.env.CIRCLE_PR_NUMBER && !process.env.DISABLE_API_TESTS, 'Multichain', () => {
// TODO temporary disable xDAI, CELO, Sokol, and Goerli
// FIXME: API-based checks should run nightly and be mocked for CI
[
'mainnet',
// 'rinkeby',
// 'goerli',
// 'xdai',
// 'sokol',
'fuse',
//'celo',
const infoRetriever = new EthInputDataInfoRetriever(
paymentAddress,
PaymentTypes.EVENTS_NAMES.PAYMENT,
'matic',
'fantom',
].forEach((network) => {
it(`Can get the balance on ${network}`, async () => {
const retriever = new EthInputDataInfoRetriever(
'0xc12F17Da12cd01a9CDBB216949BA0b41A6Ffc4EB',
PaymentTypes.EVENTS_NAMES.PAYMENT,
network,
'9649a1a4dd5854ed',
process.env[`EXPLORER_API_KEY_${network.toUpperCase()}`],
);
await expect(retriever.getTransferEvents()).resolves.not.toThrow();
});
});
Comment on lines -56 to -80
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this test. This is covered by the first test in terms of logic. The rest isn't "unit" test, but integration, and is too flaky. This is an obsolete feature and doesn't deserve the effort imo


it('can detect a MATIC payment to self', async () => {
// NB: The from and to are the same
const paymentAddress = '0x4E64C2d06d19D13061e62E291b2C4e9fe5679b93';
const paymentReference = PaymentReferenceCalculator.calculate(
'01b809015dcda94dccfc626609b0a1d8f8e656ec787cf7f59d59d242dc9f1db0ca',
'a1a2a3a4a5a6a7a8',
paymentAddress,
);

const infoRetriever = new EthInputDataInfoRetriever(
paymentAddress,
PaymentTypes.EVENTS_NAMES.PAYMENT,
'matic',
paymentReference,
process.env[`EXPLORER_API_KEY_MATIC`],
);
const events = await infoRetriever.getTransferEvents();
expect(events).toHaveLength(1);
paymentReference,
);

expect(events[0].amount).toBe('1000000000000000');
});
const events = await infoRetriever.getTransferEvents();
expect(events).toMatchObject([
{
amount: '1000000000000000',
name: 'payment',
parameters: {
block: 17182533,
confirmations: 33274506,
txHash: '0x50af07756eb07bb0eb29943cb7206d8359c829aef7f6dad50f61b488c2790c1c',
},
timestamp: 1627050587,
},
]);
});
});
Loading