Mochify Rewrite about mochify.js HOT 14 CLOSED

Woohoo 🎉 this seems to be an amazing start. I love how much simpler this is.

Some uninformed feedback after a first glance:

Zero config

The reason why I started using Mochify in ~2014 was that "it just worked ™️". I would install the package, it picked up my browserify config all by itself and it could run my tests in a real browser 💌. Compared to what was out there at the time this was truly amazing. Maybe I am biased because I still mostly refuse to abandon Browserify in 2021, but I think it's still amazing today: I get a test runner that does exactly what I want with zero config and by installing a single package only.

Playing the devil's advocate, seeing that I now have to install multiple packages and possibly provide a config file makes me feel slightly uneasy. Will zero config work too? Will there be a single package that wraps the CLI and a default driver?

Passing options to Drivers

Maybe you're still planning to do this, but it seems to me as if we'd need to be able to pass options to drivers at some point, which currently seems to be unsupported:

Custom non-@mochify drivers

Drivers seem to require to reside under the @mochify scope. Is this intentional or should there be a fallback allowing consumers to provide arbitrary drivers?

Default driver

I don't know if you ever had a look at that branch, but I ported Mochify to use playwright instead of puppeteer a while ago, which was relatively painless (but I never got around to know how the testing story should work, so I shamefully abandoned it): https://github.com/mantoni/mochify.js/tree/playwright-firefox. Considering this also adds Firefox and Webkit support I wonder if this should maybe become the default driver instead of puppeteer? Or is puppeteer a nicer option because it's more lightweight? Really not sure.

Extra deps

This might be very subjective because my $DAYJOB consists of googling for error messages of npm failing on humongous dependency trees ~70% of the time (no exaggeration unfortunately), but I am wondering if adding execa and cosmiconfig is really needed. Could this just use child_process? Could config live in package.json on the mochify key?

If time allows I think I'd like to try adding a playwright driver and also see if I can get this working with esbuild instead of Browserify next week. I'll keep you posted on the progress ✌️

from mochify.js.

it's a really important decision which driver is mentioned in the npm i command

That's a good point. We should make "the best" driver the default. If playwright gives us support for three different browsers, then I agree that it should be the one in the install instructions.

On top of the "default" npm i command, we could come up with documentation for different use cases and corrsponding install / config instructions.

how do you plan to handle versioning in the monorepo?

With npm workspaces, each of the workspace folders is a separate module that can be versioned and published. E.g. npm version patch -w cli would make a new version of @mochify/cli only. The cli has a regular dependency on @mochify/mochify. For the drivers, we should declare a peerDependency on @mochify/mochify to state compatibility. I have successfully used this approach, also with multiple major versions:

{
  "peerDependencies": {
    "@mochify/mochify": "^1 || ^2"
  }
}

from mochify.js.

I found half an hour to play around with this and I couldn't find any major issues:

• adding a playwright driver was very simple: #230
• using esbuild worked out of the box
• I ran the test suites of 2 smaller projects (~150 tests each) that are currently using Mochify 8 just fine using the rewrite branch

from mochify.js.

For what it's worth I tried implementing a jsdom driver (which would be nice as a very minimal option) over here: https://github.com/mantoni/mochify.js/tree/jsdom-driver

I'm running into something I don't fully understand though:

• jsdom evaluates all of the passed scripts just fine
• console.log calls originating from within Mocha will be swallowed and appear nowhere, thus the runner will hang polling for events infinitely
• when I sprinkle the client with random console.log calls (before and after overwriting the methods to point at write) these will work as expected (i.e. log or create an event that will then be retrieved on polling)

I remember Mocha is doing some custom console modifications, but I cannot really connect the dots here yet.

Some more debugging records here:

• the logging behavior seems to be a red herring
• the bundled tests get wrapped in a script tag and are injected into the DOM correctly, which will also execute the contents immediately
• Mocha collects suites and tests correctly
• calling mocha.run will never run any tests. The runner instance is returned correctly and doesn't really look different than when using a different driver. Nothing throws.

Found out what is going on here and fixed it in #232

from mochify.js.

Thank you for the quick feedback. Here are my thoughts on your points:

Zero config

This was my design goal with mochify at the time, but you're sort of making the point that we cannot keep it this way yourself 😄. You might want to bundle with esbuild or run tests with playwright and a pluggable design makes this possible. Having a default bundler and driver would require adding them as dependencies which makes it a rather large install again. Especially with puppeteer, which downloads chromium, and browserify that comes with quite a tree of dependencies.

Passing options to Drivers

Yes, we'd need to be able to pass options to drivers. The current state is just a proof of concept. There's a lot to do.

Custom non-@mochify drivers

Yep, it would be nice to allow "external" drivers. The mochify cli could map "puppeteer" and "selenium" to the @mochify names and, if none match, use the configured driver name as is.

Default driver

I had a brief look at your work on playwright, but didn't get to really experiment with it myself. Like with the "Zero config" argument, it might be a great default for you, but other projects might prefer puppeteer for some reason, or even only need a selenium driver. So I'd rather not add a default driver at all.

Extra deps

Sure, we could do the work ourselves. However, execa does a great job and I'd like to keep things simple. I basically stole the execa and string-argv combo from lint-staged and love how easy it was to get it working.

Another note: With the latest commits, I managed to run the sinon.js test suite like this:

../mochify.js/cli/index.js './test/**/*-test.js' --bundle 'browserify --no-detect-globals --plugin [ proxyquire-universal ] --debug' -R dot

from mochify.js.

A few more words about the "default": I probably could have made this a lot clearer, but when I said "default" I was mostly referring to what is being mentioned in the above-the-fold installation instructions in the README, which of course isn't a bundled default, but I'd argue it creates an implicit default. Right now, it reads:

npm i @mochify/cli @mochify/driver-puppeteer -D

and I would assume this means that 98% people trying it out will install this driver and 90% of users will stick to it, no matter how many other options there are. I understand it's meant to be pluggable, but many consumers won't have time and/or knowledge to make an educated decision about which driver to use, so they will stick to what they initially installed.

So what i was trying to say is: it's a really important decision which driver is mentioned in the npm i command, and being aware of the consequences will pay off.

Another question unrelated to this came to mind: how do you plan to handle versioning in the monorepo? Are drivers and the cli planned to use the same major always or could they theoretically differ and each package has its own set of compatibility guarantees?

from mochify.js.

Another random shower thought of mine:

Assuming we want to completely offload the bundling to the consumer and just have Mochify consume a bundle bundled by basically everything, why doesn't this command:

mochify './test/**/*-test.js' --bundle 'browserify --no-detect-globals --plugin [ proxyquire-universal ] --debug' -R dot

look like this instead, freeing us of having to handle the bundling command:

browserify --no-detect-globals --plugin [ proxyquire-universal ] --debug' -R dot './test/**/*-test.js' | mochify

i.e. Mochify just reads the bundled files from stdin and that's it? Is there anything we couldn't do like this? Is this complicated because of Windows (not that I have any idea about this)?

from mochify.js.

We could support your proposal, to read from stdin. I'm generally a friend of a UNIX style interface. I would still support a bundle option, especially for config files. Those lines with pipes tend to get really long and hard to understand.

from mochify.js.

I would still support a bundle option, especially for config files.

Ah, so the bundle command would go in the config file in a "proper" setup and wouldn't be passed on the command line at all?

from mochify.js.

That's the idea so far. We can allow to specify / override it on the command line though.

from mochify.js.

About reading the bundle from stdin as an option instead of passing a command: I looked into how this could work on a code level and found it slightly odd that bundling happens on library level instead of CLI level. Is there any reason you don't want to just pass a plain bundle string to the library? When used as a library in code (gulp or whatever), I would assume consumers would also want to programatically use their bundler(s) and could just pass a string or a stream that emits the bundle?

from mochify.js.

Hm, interesting point. My design goal here would be that the API and the CLI work the same way. Basically all options are a one-to-one mapping of configs that can be passed to mochify(config). Ideally calling mochify() and npx @mochify/cli does the same thing. That's why I've put the bundling into the API.

However, the API can do more than the CLI and allow the bundle to be a stream. What do you think?

from mochify.js.

Ideally calling mochify() and npx @mochify/cli does the same thing.

This makes a lot of sense considering the config file plan. Adding extra options to the API will still work.

from mochify.js.

Since we have a first version of the rewrite published, I think it's time to have more focused discussions on the individual issues. I have created a Milestone to group open issues here: https://github.com/mantoni/mochify.js/milestone/1

Please feel free to add whatever you feel is missing. Thanks a lot for all the help!

from mochify.js.