Feedback and Suggestions

[dimitrovmaksim]

Feel free to share your feedback and feature suggestions here

[fant0m98]

hello, it would be great if there is a possibility to mock the IPFS requests at the tests

[OlaStenberg]

Hey,

I think it would be good to have an assertion to check how many objects there are for an entity, I can think of at least two use cases for it.. I can't think of a better method name right now, so lets call it size(entity: string, expectedVal: number)

Check for "duplicates" when multiple objects are created

test('Transfer', () => {
  let transferEvent1 = createTransferEvent(...)
  let transferEvent2 = createTransferEvent(...)
  onTransfer(transferEvent1)
  onTransfer(transferEvent2)

  assert.size('Transfer', 2)
})

I haven't had much experience with the thegraph or matchstick, but I normally do these types of tests when I use a graph database to check that a merge haven't created a duplicate object. In this case, I don't see it happening for the same reason, I could use a function getOrCreateTransaction(id), but perhaps the event params contains a list and multiple objects should be created.

The other use case - Make sure something isn't created, for whatever reason that could happen in the mapping function onTransfer

test('Transfer', () => {
  let transferEvent = createTransferEvent(...)
  onTransfer(transferEvent)

  assert.size('Transfer', 0)
})

I only know one alternative to this, which is using assert.notInStore('Transaction', id), the issue with this is that the id creation could change and the test would still be valid.

[this-username-is-taken]

Add a README to document common compilation errors. For example:

 "Critical: Could not create WasmInstance from valid module with context: unknown import: wasi_snapshot_preview1::fd_write has not been defined"

This is shown when console.log is used, and can be resolved quickly if known otherwise almost impossible to debug.

[0xmad]

Hello,

It would be great to have coverage threshold (for instance, like jest has https://jestjs.io/docs/configuration#coveragethreshold-object) and coverage reports. Also, it will be useful if there are test hooks (beforeAll, afterAll, beforeEach, afterEach).

[dimitrovmaksim]

Hello,

It would be great to have coverage threshold (for instance, like jest has https://jestjs.io/docs/configuration#coveragethreshold-object). Also, it will be useful if there are test hooks (beforeAll, afterAll, beforeEach, afterEach).

Hey, describe and before/after hooks are in the works, if you wish you can give them a try. You can find more info here: https://github.com/LimeChain/matchstick/pull/273

[HetmanJones]

I notice the graph test always do a request to https://api.github.com/repos/LimeChain/matchstick/releases/latest ? Why is not cached?

[0xJurassicPunk]

I notice the graph test always do a request to https://api.github.com/repos/LimeChain/matchstick/releases/latest ? Why is not cached?

I agree. It would be best if it can be cached

[dimitrovmaksim]

I notice the graph test always do a request to https://api.github.com/repos/LimeChain/matchstick/releases/latest ? Why is not cached?

graph-cli 0.30.3 should now use cache file to cache the request <tests_folder>/.latest.json. The check will now be executed once per day instead of every time. Specifying a version with -v/--version will also skip the request. Using -f/--force will perform the request even if the cache has not expired.

[davidfragalaureano]

Hi folks, great work on this framework.

I have a suggestion when it comes to save entities during unit tests. It happens to me that while running my subgraphs, I forgot to set a value for an entity property that is marked as required in the graphql.schema, then when my subgraph is trying to index events, it throws an error since a required field was not set, i.e:

Entity AssetWrapped[0xbf1433f9d16b7b23302c3027b9947b3a4b4de076fcd61d3df13afedac8b5825b]: missing value for non-nullable field `state`   

My suggestion is that if there would be a way to catch this during unit tests and throw an error telling the user that the entity cannot be saved with null values for required fields. i. e:

    // listing graphql schema
    type Listing @entity {
       id: ID!
       state: String!
    }

    newListing = new Listing(listingId.toHex()); 
    newListing.save();  // this would fail as I'm not setting 'state' property in the entity before saving

[dimitrovmaksim]

Hi folks, great work on this framework.

I have a suggestion when it comes to save entities during unit tests. It happens to me that while running my subgraphs, I forgot to set a value for an entity property that is marked as required in the graphql.schema, then when my subgraph is trying to index events, it throws an error since a required field was not set, i.e:

Hey, thanks for the feedback, we have added a check for the required fields and now your test should fail if a required field is not set when saving an entity. You can try it out by adding -v 0.5.2-pre to your test command.

[KholdStare]

Hi!

I've just started using matchstick and it's a great tool. I have a suggestion, but let me know if there's already a better way.

While writing unit tests I usually don't particularly care about the specific values in events that I test the handlers with, as long as things are "right" in the store. I've found myself writing a lot of boilerplate to create these events for unittests, similar to this: https://github.com/LimeChain/demo-subgraph/blob/main/tests/gravity/utils.ts#L39-L53 . Since you're already generating code that defines the Event types, why not include a few helper functions:

export class NewGravatar extends ethereum.Event {
  // Note that types align with the types required - they're not all strings
  static fromParams(id: i32, ownerAddress: Address, displayName: string, imageUrl: string): NewGravatar {
     // ...
  }
  //...
}

Secondly, there should be a way of generating valid "example" values for the different types. E.g. Address.random(). Combining the two suggestions you could create example events very easily/quickly, without the need of any boilerplate code.

const id = randomId();
const ownerAddress = Address.random();
const event = NewGravatar.fromParams(id, ownerAddress, randomString(), randomString());

// use ownerAddress as required.

Maybe "random" is not the best name, perhaps it can be "mock". If matchstick creates random values then you'll need to output the random seed, and also allow users to override it so failures can be recreated.

Thanks! I think the first part will have the biggest impact, while the second is a nice-to-have.

[dimitrovmaksim]

Hey, there currently is a PR in graph-cli that does exactly that, generating the helper functions for each event and example tests on init and add, similarly how graph init generates the files in the src folder. https://github.com/graphprotocol/graph-cli/pull/919

I don't think generating such files or examples should be handled by matchstick itself. Matchstick's purpose is to compile and execute the test files.

[bsamuels453]

hey, really love this library. Have been using it extensively and had some feedback to share.

One of the biggest challenges we've had with complex subgraphs is setting up all the correct mocked functions using createMockedFunction.

Some event handlers may need to issue dozens of RPC calls that have to be mocked with createMockedFunction, which makes the tests for those event handlers difficult to maintain.

It would be really nice if we could do something like the following workflow:

1. Deploy our contracts to a local node
2. Configure the contracts using RPC calls in preparation for a test
3. Issue the RPC call that will trigger the event handler under test
4. RPC calls go to the local node instead of having to be mocked using createMockedFunction

Not sure if anything like this is on your radar, but it would be a massive help for more complex subgraph testing.

[dimitrovmaksim]

Hey, thank you for the feedback. Unfortunately at the moment there are no plans for new features for matchstick (we will see if this changes in the future), but we are still committed to providing support and bug-fixes

[OlaStenberg]

Hey again, thanks for adding the assert.entityCount() I suggested!

I have another suggestion, adding support for data driven tests

Jest example:

it.each([[2,4], [6,36], [8,64]])('can square %i to %s', (n, expected) => {
    expect(n*n).toBe(expected);
});

Currently there's no support for this afaik, so just added a for loop inside a test:

test(`Test stable pool token0Price/token1Price calculation`, () => {
    for (let i = 0; i < TEST_ARGS.length; i++) {
        clearStore()
        const syncEvent = createSyncEvent(TEST_ARGS[i].reserve0, TEST_ARGS[i].reserve1)

        onSync(syncEvent)

        assert.fieldEquals('Pair', PAIR.toHex(), 'token0Price', TEST_ARGS[i].expectedToken0Price.toString())
        assert.fieldEquals('Pair', PAIR.toHex(), 'token1Price', TEST_ARGS[i].expectedToken1Price.toString())

        log.debug("Test {} reserve0: {} reserve1: {}, expected token0Price: {}, token1Price: {}", [(i + 1).toString(), TEST_ARGS[i].reserve0.toString(), TEST_ARGS[i].reserve1.toString(), TEST_ARGS[i].expectedToken0Price.toString(), TEST_ARGS[i].expectedToken1Price.toString()])

    }
})

The above works, but not ideal since I have to clear the store first and do all the logging myself. If we had support for this, it could support beforeAll, afterAll, plus it could describe the input variables in the description, example:

Test stable pool calculation, with reserve0: ${reserve0} and reserve1: ${reserve1}, the expected prices are: ${price0} and ${price1}

[novaknole]

Hi all.. Thanks for this library ^__^

Since describe feature has been added, I would love to have the following feature.

describe("", () => {
   let test: string;

   beforeEach(() => {
       test = "11";
   })

   it("run", () => {
       log.warning("show - {} ", [test]);
   })
})

The following says: Not implemented: closures.

[dimitrovmaksim]

Hi, closures are not supported in AssemblyScript, so there's not much we can do about it.

[novaknole]

@dimitrovmaksim Hi,

I got couple of issues that could be turned into features or they might be already implemented and I don't know about it.

1. my mapping calls supportsInterface on an address that looks like this.

let contract = ERC721.bind(addressHere);
let result = ethereum.call(
    new ethereum.SmartContractCall(
      contract._name, // '',
      contract._address, // address,
      'supportsInterface', // '',
      'supportsInterface(bytes4):(bool)',
      [ethereum.Value.fromFixedBytes(Bytes.fromHexString('0x80ac58cd') as Bytes)]
    )
  );

  if(result != null &&
    (result as Array<ethereum.Value>)[0].toBoolean() == true)) {
    // do something
  }

when supportsInterface is called from mapping, and the function isn't mocked in test, it fails with: couldn't find mocked function. why ? can't we have supportsInterface so even if mocked function not exists, it doesn't fail ? and more importantly, if supportsInterface is called on a non-erc721, would the realtime subgraph fail or is this happening only in test ? I don't want to be mocking the supportsInterface in test and I simply want it to return null.

2. The second problem I found is one test depens on another. If I mock supportsInterface in test 1, it's also mocked in test 2 even though I didn't mock it in test 2.

it("1", () => {
   // I create mocked function here
})

it("2", () => {
   // problem, the mocked function is still in cache in test 2
   // and this is really bad.
})

is there clearCache function ? and if so, should I be calling it in afterEach ? still not ideal.

3. I got 5 tests written. Each of them tests something about ERC721. I end up having the following in all 5 tests.

// check ERC721Contract entity
    assert.fieldEquals('ERC721Contract', tokenId, 'id', tokenId);
    assert.fieldEquals('ERC721Contract', tokenId, 'name', 'name');
    assert.fieldEquals('ERC721Contract', tokenId, 'symbol', 'symbol');
    assert.entityCount('ERC721Contract', 1);

    // check ERC721Balance entity
    assert.fieldEquals('ERC721Balance', balanceId, 'id', balanceId);
    assert.fieldEquals('ERC721Balance', balanceId, 'token', tokenId);
    assert.fieldEquals('ERC721Balance', balanceId, 'dao', daoId);
    assert.fieldEquals('ERC721Balance', balanceId, 'tokenIds', '[4, 12]');
    assert.fieldEquals('ERC721Balance', balanceId, 'lastUpdated', timestamp.toString());
    assert.entityCount('ERC721Balance', 1);

The reason I wanted the feature of having let variables in describe block is, then I'd be able to have the chance that each test just sets the variables (tokenId, balanceId, daoId, timestamp), and the above assertions would go in afterEach and wouldn't be duplicated. Isn't there really a solution for this ?

Thank you so much

[dimitrovmaksim]

1. If you have a contract call in your mappings it will always try to execute it and because the tests are executed "offline", i.e. the subgraph is not "connected" to the chain, the call will always fail anyways and code will panic, so you have to mock it.
2. Yeah the mocked functions are persisted. In the next test you will need to mock it again, or if you want it to fail, you can mock a revert https://thegraph.com/docs/en/developing/unit-testing-framework/#mocking-contract-calls. We can probably add a .once() method so the mock is executed only once. We will have to take a look if we can adjust the scoping somehow. Also it's not a good practice to have tests depend on the result of a previous tests.
3. It's a limitation of the programming language. Alternativelly you can extract the assertions into a function and pass the expected values as arguments and call it at the end of each test.

[novaknole]

@dimitrovmaksim

thanks for such a detailed answer.

1. i guess in real time subgraph(not tests, but connected to real chain) even if supportsInterface does not exist on a contract, the subgraph wont fail and contract call will just return null right ?
2. Exactly, currently the way it works it is a really bad practice. Test 2 depends on test 1 and in every test that comes after test 1, i should be mocking the contract call with false values. Not ideal.

[dimitrovmaksim]

@dimitrovmaksim

thanks for such a detailed answer.

1. i guess in real time subgraph(not tests, but connected to real chain) even if supportsInterface does not exist on a contract, the subgraph wont fail and contract call will just return null right ?

I guess it will revert - https://thegraph.com/docs/en/developing/assemblyscript-api/#handling-reverted-calls. You can mock a reverting function (check the link in my previous comment) if you want to test that.

2. Exactly, currently the way it works it is a really bad practice. Test 2 depends on test 1 and in every test that comes after test 1, i should be mocking the contract call with false values. Not ideal.

If you trigger the handler in each test and it performs the contract call, you have to mock the call anyways because it will be performed by the subgraph each time the handler is triggered. As I mentioned if you don't mock the function, and even if we make mocking the function to not be required, the code will just panic, because it can't connect to the network.

[novaknole]

@dimitrovmaksim Good thing. It doesn't revert in real time subgraph which is great. So I still need to be mocking this in tests.

If you trigger the handler in each test and it performs the contract call, you have to mock the call anyways because it will be performed by the subgraph each time the handler is triggered. As I mentioned if you don't mock the function, and even if we make mocking the function to not be required, the code will just panic, because it can't connect to the network.

The case I have is my mapping doesn't always calls supportsInterface contract call by ethereum.call. The logic that I have is my mapping checks if the event.params.token is ERC721. Problem is sometimes I have to pass ERC20 token from the tests instead of ERC721(I got some requirements like this), and even if I pass ERC20 address, I still need to be mocking supportsInterface so my tests doen't fail with can't find mocked function. If ethereum.call would return null, instead of reverting in this case, then it would be awesome. I don't really know. Test doesn't fully resemblance to how it works on mainnet subgraph (as I said, on mainnet, ethereum.call doesn't revert even if supportsInterface doesn't exist on a contract.

[dimitrovmaksim]

But calling supportsInterface is also a contract call, if you do it everytime the handler is triggered, you have to mock that call also. If we let contract calls to default to null/revert, users may not realize that they need to mock contract functions and may lead to getting weird results and frustrating experience.

[novaknole]

@dimitrovmaksim It's understandable, but note that the way I call it in mapping is that I check its returned value and check if it's not null. If every user does this, then even if they don't mock it(forgot or whatever), they would get null value and would realize that call wasn't successful. It's just on mainnet, even if contract doesn't contain the function, it doesn't revert, but returns me null and thats how I know call wasn't successful.

[dimitrovmaksim]

We can't assume everyone knows or is experienced enough.

[valle-xyz]

Hey! Awesome library!

I realized I am writing a lot of boilerplate code to create events. So I thought it could be nice to simplify the whole event creation to something like:

callHandler("handleSpecificEvent", [stringParam1, intParam2, fooParam3]);

The callHandler could then check the params of the event that is needed for the specific handler, and then create this event, try to assign the values of the array and call the handler with the event. If it fails, or even if the array length is too large, it could fail at runtime / test time. This will still decrease the time to write tests tremendously.

What do you think?

UPDATE: Created this PR, reduces boilerplate code and makes your awesome library even more accessible: https://github.com/LimeChain/matchstick-as/pull/55

[mathewmeconry]

Hey @dimitrovmaksim Thanks for the lib. works really well.

I would like to also test if a certain dynamic template is created. Assuming we have this code snipped:

export function handleMembershipContractAnnounced(
  event: MembershipContractAnnounced
): void {
  let token = event.params.definingContract;
  let packageEntity = TokenVotingPlugin.load(event.address.toHexString());

  if (packageEntity) {
    let contract = fetchERC20(token);
    if (contract) {
      packageEntity.token = contract.id;

      packageEntity.save();

      let context = new DataSourceContext();
      context.setString('pluginId', event.address.toHexString());
      GovernanceERC20.createWithContext(event.params.definingContract, context);
    }
  }
}

I would love to check if GovernanceERC20 got created with the proper address and context.

if it is already possible could you tell me how?

[georg-getz]

Hi @mathewmeconry As of yet testing options for dynamic templates are very limited. You can read this short section to see what's currently possible, but things like create() and createWithContext() just do nothing. The reason for that is that they would require a lot (and I mean a lot) of additional mocking and simulating on our end and we are a bit stretched on resources right now.

That being said we are tracking the frequency of bigger improvement suggestions such as this one and will talk with The Graph if interest grows.

[monitz87]

Hey @dimitrovmaksim Thanks for the lib. works really well.

I would like to also test if a certain dynamic template is created. Assuming we have this code snipped:

export function handleMembershipContractAnnounced(
  event: MembershipContractAnnounced
): void {
  let token = event.params.definingContract;
  let packageEntity = TokenVotingPlugin.load(event.address.toHexString());

  if (packageEntity) {
    let contract = fetchERC20(token);
    if (contract) {
      packageEntity.token = contract.id;

      packageEntity.save();

      let context = new DataSourceContext();
      context.setString('pluginId', event.address.toHexString());
      GovernanceERC20.createWithContext(event.params.definingContract, context);
    }
  }
}

I would love to check if GovernanceERC20 got created with the proper address and context.

if it is already possible could you tell me how?

Fwiw, I really need this as well. I have a handler that actually doesn't interact with the store at all, but rather only creates a data source from a template. The only thing I could test is that the template is created with the correct arguments, and that isn't currently possible. The handler also makes a call to a contract function, so I guess I also could test for that, but that also isn't possible for similar reasons.

A very powerful solution would be to support stubbing and mocking in a more general fashion. I imagine this is something you already have in mind, and I assume it's way easier said than done. Still, as a start, just adding the possibility to do something like

const mockedFunction = createMockedFunction(...).withArgs(...).returns(...);

// ...

assert.called(mockedFunction);

and/or

const mockedFunction = dataSourceMock.createWithContext(...).withArgs(...).returns(...);

// ...

assert.called(mockedFunction);

Would go a long way