near / near-workspaces-js

Write tests once, run them both on NEAR TestNet and a controlled NEAR Sandbox local environment
https://near.github.io/near-workspaces-js/
GNU General Public License v3.0
42 stars 21 forks source link

Drop near-workspaces-ava #111

Closed ailisp closed 2 years ago

ailisp commented 2 years ago

As discussed with @MaximusHaximus, near-workspaces is considered as a library. User should be able to fit near-workspaces into the test runner (jest, mocha, ava) of their choice. near-workspaces hence won't provide any test runner features.

Therefore, near-workspaces-ava will be dropped. Users can still write test with ava and we'll provide examples. Example test will be generated with near-workspaces-init ava. Please comment if you have different opinions.

willemneal commented 2 years ago

We had arrived at featuring ava because jest has concurrency issues. Furthermore, the ava library provides a way to remove boilerplate from ava tests as well as the configuration needed. This provides the fastest way for people to get started with using the workspace with minimal setup required.

User should be able to fit near-workspaces into the test runner (jest, mocha, ava) of their choice. near-workspaces hence won't provide any test runner features.

This is already true since near-workspaces doesn't force you to use ava.

chadoh commented 2 years ago

I originally pushed back on adding near-workspaces-ava, but from our user-testing, it seemed like JS newbies of all stripes really appreciated having a way to get started quickly with some suggested conventions. near-workspaces-ava lets people start using near-workspaces-js in one command using a fast, bug-free concurrent test-runner and full TypeScript support.

chadoh commented 2 years ago

The README already includes a prominent "Manual Install" section that explains how to use the library without AVA.

Screen Shot 2022-02-14 at 15 36 24

I'm not sure how making the library super fast to get started with makes it not-library-like.

ailisp commented 2 years ago

I think @willemneal and @chadoh 's points make sense. near-workspaces-ava provide a good opinioned setup, while advanced users can still start from low level near-workspaces-js.

@MaximusHaximus What do you think?

ailisp commented 2 years ago

With a follow up discussion with @MaximusHaximus , Daryl concludes that above proposed benefits are not necessary to a near-workspaces-ava. For "user need quick conventions with minimal setup", it can also provided by example test files like this:

before(async t => t.context.workspace = await Workspace.init)

beforeEach(async t => t.context.currentWorkspace = await Workspace.fork())

test('Root gets null status',  async t => {
  let {root, contract} = t.context.currentWorkspace.contracts();
  const result: null = await contract.view('get_status', {
    account_id: root
  });
  t.is(result, null);
});

(need some work to modify workspaces-js apis, but that's about five lines of setup)

And near-workspaces-ava have some drawbacks. For example, it makes ava test.skip no longer work. IDEs like webstorm can no longer statically analysis and mark tests to run in the editor. Given that 1) raw workspaces-js with examples can be as convenient as ava 2) doesn't require us to catch up with work that test runner framework already implemented. 3) due to a bug #116 , local near-workspaces-ava cannot be tested with some hacks: https://github.com/near/near-sdk-js/tree/master/examples/project#test. The problem doesn't exist, if use ava directly. 4) In a real world project, if use near-workspaces-ava, workspace will be fork too many time and run out of ram in ci environment. With raw approach, user can control when to fork, when to reuse.

Therefore I agree that drop near-workspaces-ava and maintain examples is the better approach.