search

SWI-Prolog / packages-pengines

Pengines: Prolog engines
11 stars 13 forks source link

Consolidation of Node port, extensive test cases and a better way to run tests in browser #41

Closed rla closed 5 years ago

rla commented 5 years ago

This Pull Request adds the following changes:

The Node port simply replaces jQuery with najax at runtime when the environment is not browser. This keeps 100% backwards compatibility in browsers. The changes to pengines.js are quite trivial.

Tests in the browser are now run using the headless Chromium. The test cases are in the file test_js/tests.js. The change adds package.json file which is required for publishing the pengines package to NPM. This file also specifies the Chromium version and makes it possible to automatically install it with a single command.

The tests are still ran through test_js.pl. There is a small section on testing in the README file. All necessary dependencies are fixed down to patch version so it's unlikely that the current setup of tests will go bad itself with toolset updates.

I have agreed with the current pengines package owner on NPM to re-publish the package once the PR gets merged. The NPM package gets major semver upgrade so anyone using the old CoffeeScript one still have working code.

JanWielemaker commented 5 years ago

Whow! please give me some time to review ... I'm working on too many threads concurrently right now ...

rla commented 5 years ago

I'm not in hurry but I agree that the PR changeset came out a bit large. At first I only wanted to make pengines.js to use najax when the runtime is Node and had a separate set of test cases for it. Then I realized that I need some more things to actually publish it as a NPM package. Finally I decided to create uniform tests for both envs.

I have more ideas what to improve, mostly the documentation for JavaScript API users, especially on the practical use cases pointed out in https://groups.google.com/forum/#!topic/swi-prolog/JnZmQfYkAUY but I would leave them to another PR.

JanWielemaker commented 5 years ago

Thanks! The change may be large, but was easy to grasp. An official maintained npm would be great!

rla commented 5 years ago

I have published a new version to NPM using the merged codebase. It would be nice if you made an account on NPM too, so you could update it as well when needed. https://www.npmjs.com/signup

I then need your username to give ownership.

JanWielemaker commented 5 years ago

I created an account (janwielemaker). I see Anne picked this up as well though. Connect me as you see fit.

rla commented 5 years ago

I added both accounts. Before republishing, the version number (semver) in package.json should be increased.

JanWielemaker commented 5 years ago

Thanks. Good to see a simple to install version coming from a single source. Not much changes at this level, so I expect few updates.

Anniepoo commented 5 years ago

I made a swiprolog account to publish https://github.com/Anniepoo/node-red-pengines should we use janwielemaker or that. Jan, sent you credentials.

rla commented 5 years ago

All owners have the same power so you can add others too. I'm not sure if Jan wants to maintain the Node-RED Pengines interface package. On the Pengines package I added both.