Skip to content
This repository has been archived by the owner on Mar 13, 2023. It is now read-only.

Add tests #16

Open
tony-go opened this issue Jun 20, 2022 · 6 comments
Open

Add tests #16

tony-go opened this issue Jun 20, 2022 · 6 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects

Comments

@tony-go
Copy link
Member

tony-go commented Jun 20, 2022

We'd like to have tests on this repository 馃И

As much as possible we'd like to use:

  • tape or node-tap (node-tap is my new goto)
  • have two scripts
    • one for running all tests once
    • one for running tests in watch mode
  • execute the first script in the CI on node 18.x and the LTS.
  • coverage is a plus
@tony-go tony-go added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 20, 2022
@PierreDemailly
Copy link
Member

I'm on it

PierreDemailly added a commit to PierreDemailly/vis-network that referenced this issue Jun 20, 2022
PierreDemailly added a commit to PierreDemailly/vis-network that referenced this issue Jun 20, 2022
PierreDemailly added a commit to PierreDemailly/vis-network that referenced this issue Jun 20, 2022
fraxken pushed a commit that referenced this issue Jun 22, 2022
* test: add ut for utils (#16)

* chore: add test:watch script

* refactor: separate dependencies imports (#16)

* chore: add node.js github workflow (#16)

* refactor: remove useless config

* fix: install deps with npm i instead npm ci

* fix: add c8 due to broken coverage
@tony-go
Copy link
Member Author

tony-go commented Sep 6, 2022

Hey @PierreDemailly 馃憢

Thanks for this amazing work. 馃憦

How goes this issue? Do you need any support?

@PierreDemailly
Copy link
Member

Hi @tony-go
network.js is the last file to test.
I remember tried to test it but it thrown due to missing browser API and/or others things, I will look again and give more details

@tony-go
Copy link
Member Author

tony-go commented Sep 19, 2022

@PierreDemailly awesome ^^

Do you think that you could add a list of stuff you did and what is missing?

Thanks again for this amazing job 馃憦

@PierreDemailly
Copy link
Member

Hi @tony-go 馃憢
So I've tried again to test NodeSecureNetwork but i'm stuck.
There was a problem with ESM:
Capture d鈥檈虂cran 2022-11-11 a虁 16 11 43
I've tried multiple things to make it works, the only thing that seem to works is:
Capture d鈥檈虂cran 2022-11-11 a虁 16 09 08
I wonder if it is an "anti-pattern" without deep-import / tree-shaking, not sure..
Then Network is made for browser and use multiple API we don't have in Node (that make crash the process):
I've 'mock' it (adding JSDOM needed APIs to global), it seem to be a bad practice -> WDYT ?

import crypto from "node:crypto";
import { JSDOM } from "jsdom";

const { window } = (new JSDOM(`
    <div id="network-graph"></div>
    <div id="network-loader"></div>
`));
const { document } = window;

global.window = window;
global.navigator = window.navigator;
global.document = document;
global.crypto = crypto;
global.Element = window.Element;
global.HTMLCanvasElement = window.HTMLCanvasElement;

I've also needed to add canvas to the dev deps: jsdom/jsdom#3042 (comment)

I've made my first test: OK.

tap.test("it should throw when providing unknown theme", (t) => {
    try {
        new NodeSecureNetwork(nsDataSet, { theme: "AZJ001" });
    }
    catch (e) {
        t.same(e, new Error("Unknown theme AZJ001. Theme value can be LIGHT or DARK"));
    }
    finally {
        t.end();
    }
});

So I've tried somethings like this:

t.equal(network.highlightEnabled, false, "highlight is disabled by default");
network.neighbourHighlight({ nodes: [0] });
t.equal(network.highlightEnabled, true, "highlight has been enabled");

(https://github.com/NodeSecure/vis-network/blob/main/src/network.js#L173)

But if I call any method from the NodeSecureNetwork, always this error:
Capture d鈥檈虂cran 2022-11-11 a虁 18 08 24

And now, I'm stuck.
I'm not sure how to handle Missing Browser API & events (Network.on(), Network.stabilize(), Network.start/stopSimulation()...).

Maybe I should completely mock Network ?
I also would see e2e tests, WDYT ?

@tony-go
Copy link
Member Author

tony-go commented Nov 17, 2022

I saw that you already add a few test here: main...PierreDemailly:vis-network:feat/add-tests-v1

Maybe you should raise a PR, and then we could merge it.

Also if some classes or functions are too broad maybe should we think about the e2e tests as you mentioned.

@fraxken fraxken added this to Backlog in Roadmap Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Roadmap
Backlog
Development

No branches or pull requests

2 participants