Add parameters to `loadFixture`

[VVander]

Hello! I noticed in the hardhat-network-helpers package, loadFixture() doesn't allow parameters to be passed to fixtures. I use a parameter on my fixture to dictate the initial state for tests so that I don't have to duplicate code.

This is a relatively simple change, so I went ahead and altered loadFixture() to pass optional parameters via another generic type P. Although this works for my use case, I haven't tested it extensively otherwise.

import type { SnapshotRestorer } from "./helpers/takeSnapshot";

import {
  FixtureAnonymousFunctionError,
  FixtureSnapshotError,
  InvalidSnapshotError,
} from "./errors";

type Fixture<T, P> = (fixtureParameters?:P) => Promise<T>;

interface Snapshot<T, P> {
  restorer: SnapshotRestorer;
  fixture: Fixture<T, P>;
  data: T;
}

let snapshots: Array<Snapshot<any, any>> = [];

/**
 * Useful in tests for setting up the desired state of the network.
 *
 * Executes the given function and takes a snapshot of the blockchain. Upon
 * subsequent calls to `loadFixture` with the same function, rather than
 * executing the function again, the blockchain will be restored to that
 * snapshot.
 *
 * _Warning_: don't use `loadFixture` with an anonymous function, otherwise the
 * function will be executed each time instead of using snapshots:
 *
 * - Correct usage: `loadFixture(deployTokens, deployTokensParameters)`
 * - Incorrect usage: `loadFixture(async (parameters) => { ... })`
 */
export async function loadFixture<T, P>(fixture: Fixture<T,P>, fixtureParameters?: P): Promise<T> {
  if (fixture.name === "") {
    throw new FixtureAnonymousFunctionError();
  }

  const snapshot = snapshots.find((s) => s.fixture === fixture);

  const { takeSnapshot } = await import("./helpers/takeSnapshot");

  if (snapshot !== undefined) {
    try {
      await snapshot.restorer.restore();
      snapshots = snapshots.filter(
        (s) =>
          Number(s.restorer.snapshotId) <= Number(snapshot.restorer.snapshotId)
      );
    } catch (e) {
      if (e instanceof InvalidSnapshotError) {
        throw new FixtureSnapshotError(e);
      }

      throw e;
    }

    return snapshot.data;
  } else {
    const data = await fixture(fixtureParameters);
    const restorer = await takeSnapshot();

    snapshots.push({
      restorer,
      fixture,
      data,
    });

    return data;
  }
}

[fvictorio]

I agree that this would be useful; the problem is how to handle different calls to loadFixture with different params. With your change (if I'm understanding it correctly), when you do this:

await loadFixture(myFixture(1))

// ...

await loadFixture(myFixture(2))

the second call will use the state from the first one, which is incorrect.

We could add the parameters to the "cache key", but this would mean that all the parameters should be comparable, which makes things more complex.

This is a valid feature request though, thanks for opening it.

[VVander]

...the second call will use the state from the first one, which is incorrect.

Good catch! I had a feeling there were some cases I missed.

We could add the parameters to the "cache key", but this would mean that all the parameters should be comparable, which makes things more complex.

Comparable parameters works fine for my use case, since I'm just passing an object full of booleans like this:

export type SetupData = { 
        protocol: Protocol, 
        signers: Signers
}

type FixtureParameters = {
	initialize: boolean,
	setupWhitelist: boolean,
	etc: boolean
}

export async function protocolFixture(
	parameters: {
		initialize: true,
		setupWhitelist: false,
		etc: false
	}): Promise<SetupData> {
	
	const protocol = await deployProtocol(); 
	const signers = await createSigners();

	if (parameters.initialize) initializeProtocol(protocol);
	if (parameters.setupWhitelist) setupWhitelist(protocol);
	if (parameters.etc) doSomethingElse();

	return { protocol, signers } ;	
}

And here's the modified loadFixture.ts with parameters added to the snapshot key:

import type { SnapshotRestorer } from "./helpers/takeSnapshot";

import {
  FixtureAnonymousFunctionError,
  FixtureSnapshotError,
  InvalidSnapshotError,
} from "./errors";

type Fixture<T, P> = (fixtureParameters?:P) => Promise<T>;

interface Snapshot<T, P> {
  restorer: SnapshotRestorer;
  fixture: Fixture<T, P>;
  parameters: P
  data: T;
}

let snapshots: Array<Snapshot<any, any>> = [];

/**
 * Useful in tests for setting up the desired state of the network.
 *
 * Executes the given function and takes a snapshot of the blockchain. Upon
 * subsequent calls to `loadFixture` with the same function, rather than
 * executing the function again, the blockchain will be restored to that
 * snapshot.
 *
 * _Warning_: don't use `loadFixture` with an anonymous function, otherwise the
 * function will be executed each time instead of using snapshots:
 *
 * - Correct usage: `loadFixture(deployTokens, deployTokensParameters)`
 * - Incorrect usage: `loadFixture(async (parameters) => { ... })`
 */
export async function loadFixture<T, P>(fixture: Fixture<T,P>, parameters?: P): Promise<T> {
  if (fixture.name === "") {
    throw new FixtureAnonymousFunctionError();
  }

  const snapshot = snapshots.find((s) => s.fixture === fixture && s.parameters === parameters);

  if (snapshot !== undefined) {
    try {
      await snapshot.restorer.restore();
      snapshots = snapshots.filter(
        (s) =>
          Number(s.restorer.snapshotId) <= Number(snapshot.restorer.snapshotId)
      );
    } catch (e) {
      if (e instanceof InvalidSnapshotError) {
        throw new FixtureSnapshotError(e);
      }

      throw e;
    }

    return snapshot.data;
  } else {
    const { takeSnapshot } = await import("./helpers/takeSnapshot");
    const data = await fixture(parameters);
    const restorer = await takeSnapshot();

    snapshots.push({
      restorer,
      fixture,
      parameters,
      data,
    });

    return data;
  }
}

I still haven't tested this very much, but my tests are still passing at least. Would be happy to submit a PR, but I'm having trouble getting the repo to compile locally -- probably a dependency issue.

[fvictorio]

If I'm understanding the code correctly, this will work if parameters is a number or string, but it won't if it's an array or object, because it's being compared by reference.

Maybe the way to go here is to add a simple isEqual function that compares the values, and throws if they are not comparable (say, if any of the values, or nested values, is a function).

It's also possible to type this, but it's a bit of a pain in the ass.

function isEqual(a: unknown, b: unknown) {
  if (a === null) {
    return b === null;
  }

  if (a === undefined) {
    return b === undefined;
  }

  if (typeof a === "function" || typeof b === "function") {
    throw new Error("only comparable values are supported");
  }

  if (typeof a === "number" || typeof a === "bigint" || typeof a === "string") {
    return a === b;
  }

  if (Array.isArray(a)) {
    if (!Array.isArray(b)) {
      return false;
    }
    if (a.length !== b.length) {
      return false;
    }

    return zip(a, b).every((x, y) => isEqual(x, y));
  }

  // etc.
}

Just off the top of my head, I haven't tested this.

[VVander]

Ahh another good catch! The prevalence of anonymous objects is really throwing me off here. I'm used to C# where I can limit generic parameters like this: public myFunction<T>() where T is IEquatable<T>. X extends Y in TS is obviously similar to where X is Y in C#, but I don't know of the TS equivalent of IEquatable. For now, I've just manually added the function like you suggested (see commit ref above), though this doesn't appear to change anything since my tests were already passing. Maybe I'm still missing something? :shrug:

I'm going to move on, but I'll keep monitoring and update if I see anything odd. Thanks for considering this feature @fvictorio !

[fvictorio]

but I don't know of the TS equivalent of IEquatable

There isn't something like that AFAIK. Same thing for serializable types. It's really annoying to not have those things, but oh well.

though this doesn't appear to change anything since my tests were already passing. Maybe I'm still missing something?

Your previous snippet probably works in the sense of not crashing, but I think it would just always call the fixture function instead of leveraging snapshots. You can add a console.log call to check if that's the case or not.

Thanks for considering this feature

Thank you for helping me think about it~ This is something we'll probably want to add in the future, and this discussion is going to be useful.

[SvenMeyer]

@fvictorio Would be great to have an "official solution". A lot of code and setup work could be made much less redundant if a parameter (or more than one) could be passed to loadFixture

• Even if all the different parameterised versions will only be executed once, it would be better to manage.
• Easy and ready to re-use the setup if needed later again

@VVander @AlissonRS How is your experience with your approach , (still) working ?

[AlissonRS]

@SvenMeyer I dropped using that approach, it doesn't cache fixtures properly, so for now I ended up either duplicating fixtures (for different params) or just doing customization in the test itself.

[VVander]

Yep, we've also dropped the approach completely in favor of the ugly solutions AlissonRS listed :(

Hopefully this gets implemented one day. Maybe you can give this link to the EF and ask for a grant to hire more @fvictorio ? ;)

[Amxx]

For the record, we also run into a situation where we would love to pass objects to the fixture.

At some point we even did

beforeEach(async function () {
    await loadFixture(fixture.bind(this));
});

before realizing this was breaking.

[haydenyoung]

I have run into a similar problem, but...

Playing devil's advocate; wouldn't passing parameters invalidate the concept of a "fixture" I.e. a fixed object or state? Isn't the purpose of a fixture to provide a fixed outcome? If one is passes various params, I would imagine a fixture is no longer "fixed" so passing params feels like a programmer convenience rather than a fundamental improvement.

If fixture redundancy is the problem, perhaps there is a different approach to setting up the fixtures as opposed to modifying how fixtures are loaded. Not sure what this solution looks like though.

[SvenMeyer]

@haydenyoung In priciple you are right I think ;-) However, most of the time I would like to have more like a "template" functionality, so all the setup work, but for another round of tests maybe just with one parameter changed. Not sure if fixtures is even a good solution for that, maybe they should stay fixed (in contrast to my initial post). The only advantage I would see is that fixtures are a bit faster to setup a certain scenario than doing it all over again in code ... although the later would give exactly the desired functionality of a parameterised setup.

[haydenyoung]

@SvenMeyer It is an interesting conundrum; I want to know my fixtures are producing consistent results, hence the fixture must produce a known state every time. But, does this generate code duplication and, hence, redundancy?

A template sounds like a good approach.

function fixture1a() {
  const params1a = {}
  return fixture1Template(params1a)
}

function fixture1b() {
  const params1b = {}
  return fixture1Template(params1b)
}

function fixture1Template(params) {
  // heavy lifting
  return fixture1
}

Having each fixture (1a, 1b) "fixed" allows me to quickly evaluate my test setups because I should be able to easily see what contract state is being fed into my tests. And, because each fixture encapsulates a known set of params, I don't violate the concept of "fixed".

I don't know, maybe I'm misunderstanding the problem but a templating concept seems like a good fixture pattern.

Not sure how this works with with HH caching. My understanding is that fixtures are just run against a snapshot of the local chain rather than doing another round of contract deployments?

[fvictorio]

My understanding is that fixtures are just run against a snapshot of the local chain rather than doing another round of contract deployments?

Correct. And we use the function "reference" as the cache key, basically (which is a bit weird, but works, and it's the reason to forbid anonymous/inline functions).

The main problem with adding parameters is that we should include the values of those parameters as part of the "cache key", which opens a huge can of worms. It's easy for primitive values, like strings and numbers, but if you pass, say, [1, 2, 3], things get more complicated (you certainly don't want to use its reference in this case).

The bottom line is that handling this in the general case is hard. We could perhaps only accept primitive values and throw if something else is used, but that seems like an ugly middle ground.

One temporary workaround that might be good enough is to write a "fixture generating function", and handle the cache yourself. Something like:

let fixtures = {}
function generateFixture(n: number) {
  if (fixtures[n] === undefined) {
    fixtures[n] = function myFixture() {
      // normal fixture that uses `n` here
    }
  }

  return fixtures[n]
}

(give or take, I haven't tried it).