convert test runners to ocaml

[patricoferris]

This PR converts the three vscode-test runners to OCaml by adding a few extra bindings to the various JS packages. This is in partial fulfilment of #393 (it doesn't bind mocha and the like, this could be a follow-up PR perhaps or I can revisit this PR next week to add that).

cc: @tmattio

[tmattio]

It's a shame we have to add a dependency and bindings to the library for a single function. Would it be easy to implement copySync with of standard NodeJS functions?

[patricoferris]

Thanks for the quick review :))

https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy/copy-sync.js 🤷

To reduce the number of files perhaps bundling it into the existing FS bindings might be a good idea seeing as they're already an altered form of the "true" Node FS module ?

[tmattio]

To reduce the number of files perhaps bundling it into the existing FS bindings might be a good idea seeing as they're already an altered form of the "true" Node FS module ?

Sure, that sounds good 🙂

[patricoferris]

Actually, unless I'm mistaken, wouldn't this require an extra dependency for the package as fs-extra is only a dev dependency at the moment https://github.com/ocamllabs/vscode-ocaml-platform/blob/master/package.json#L966 ? Maybe not worth it then?

[mnxn]

Do we even need this dependency?

I noticed you added Fs.Sync.copy, can't that be used instead of Fs_extra.copy_sync?

[patricoferris]

IIUC Fs_extra.copy_sync is used because it copied directories (not just that it is synchronous).

[mnxn]

Okay in that case I personally think we should leave it as-is.

It's a shame we have to add a dependency and bindings to the library for a single function.

@tmattio: It's not adding a dependency since the JS version of the tests also used this library.
We can remove the dependency later in another PR if necessary.

unless I'm mistaken, wouldn't this require an extra dependency for the package as fs-extra is only a dev dependency at the moment

That's right. That means that keeping the bindings to fs-extra separate would not require us to add it as a runtime dependency.

[tmattio]

Thanks @patricoferris, this is great!

This is in partial fulfilment of #393 (it doesn't bind mocha and the like, this could be a follow-up PR perhaps or I can revisit this PR next week to add that).

Up to you, we can leave it until next week open if you want to continue working on it, but I'm fine with merging as-is.

[ulugbekna]

To reduce the number of files perhaps bundling it into the existing FS

bindings might be a good idea seeing as they're already an altered form of the "true" Node FS module Could you, please, then leave a comment explaining why you're doing so to avoid confusion in the future?

…

On Tue, 2 Nov 2021 at 17:14, Thibaut Mattio ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src-bindings/fs-extra/fsextra_stub.js <#762 (comment)> : > @@ -0,0 +1 @@ +joo_global_object.fsExtra = require('fs-extra') To reduce the number of files perhaps bundling it into the existing FS bindings might be a good idea seeing as they're already an altered form of the "true" Node FS module ? Sure, that sounds good 🙂 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#762 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4YR65GWRMPLBOOU2PVGBDUKAE7XANCNFSM5HG2APCQ> .

[rgrinberg]

@patricoferris is this PR ready?

[patricoferris]

Sorry for the delay doing some more testing locally first to debug https://github.com/ocamllabs/vscode-ocaml-platform/runs/4082355161?check_suite_focus=true