Open issues for each of the many cheeky it.skip() instances in the codebase.

[iAmMichaelConnor]

Several tests are skipped, but without explanation. We should work through and understand why, and open issues (and add a comment to the code with the issue number).

[jeanmon]

Confirmed tests which failed locally

• acir-simulator, index.test.ts, 'should run the transfer function without enough sender balance' (Task 1588)
• circuits.js, rollup_wasm_wrapper.test.ts, 'calls base_rollup__sim' (Task 1586)
• circuits.js, rollup_wasm_wrapper.test.ts, 'calls root_rollup__sim' (Task 1586)
• e2e_nested_contract.test.ts, 'parent manually calls child' test suite with 3 following cases failing:
  'enqueues multiple public calls', 'enqueues multiple public calls with nested public calls', 'reads fresh value after write within the same tx' (Task 1587)

Performance Test

• merkle_tree, sparse_tree.test.ts, 'measures time of inserting 1000 leaves at random positions for depth 254'

Placeholder/Unimplemented Tests

• sequencer-client, l1-publisher.test.ts, 'waits for fee distributor balance' AND 'fails if contract is changed underfoot' (Task 1600)

REMARK: All other skipped tests were re-enabled as part of the PR #1583

[jeanmon]

@spalladino Should I open a ticket for the two un-implemented tests in l1-publisher.test.ts? Or should I remove them from the code?

[jeanmon]

@iAmMichaelConnor @benesjan Any idea how we should handle the performance test mentioned above? I added a comment in the code that we skip it because it is a performance test. Do we have any plan/guidelines on how to deal with performance tests at Aztec?
@iAmMichaelConnor Are you fine if we keep this test skipped?

[iAmMichaelConnor]

I don't think we do have a plan for dealing with performance / benchmarking tests. Barretenberg contains a lot of benchmark tests, but that's because it's been optimised for historic products. We haven't reached a point where we want to optimise the typescript code yet. If all it's testing is speed (and there are other tests which are ensuring leaf insertion logic is correct), then it's probably fine to skip this test (especially if it takes a significant amount of time).

[spalladino]

@jeanmon let's open issues for those, and link to them in these TODOs:

aztec-packages/yarn-project/sequencer-client/src/publisher/l1-publisher.ts

Line 193 in c5f734a

// TODO: Check fee distributor has at least 0.5 ETH.

aztec-packages/yarn-project/sequencer-client/src/publisher/l1-publisher.ts

Line 199 in c5f734a

// TODO: Fail if blockchainStatus.nextBlockNum > thisBlockNum.