Skip to content

Add Mocha tests - #3815

Merged
gorhill merged 1 commit into
gorhill:masterfrom
mjethani:mocha-tests
Aug 13, 2021
Merged

Add Mocha tests#3815
gorhill merged 1 commit into
gorhill:masterfrom
mjethani:mocha-tests

Conversation

@mjethani

@mjethani mjethani commented Aug 13, 2021

Copy link
Copy Markdown
Contributor

Work in progress.

This adds the Mocha testing framework.

Run make test.

"devDependencies": {
"eslint": "^7.32.0"
"eslint": "^7.32.0",
"mocha": "^9.0.3"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gorhill I'm introducing the Mocha framework here.

I have left the test.js file as it is for now, but in the final version of this patch, if you like the idea, we can remove code from that file.

Please see other inline comments.

let engine = null;

describe('SNFE', () => {
function fetch(listName) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is code taken from ../test.js.

function testSNFE(engine) {
let result = 0;

// Tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are supposed to split up the tests but in this initial patch they are all one test.

type: 'stylesheet'
});
if ( result !== 0 ) {
engine.toLogData();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed console.log() because that's not the Mocha way of doing it anyway. We need to compare the actual output against the expected output and throw an error via assert.deepEqual().

}
}

beforeEach(async () => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the beforeEach() block we have code that should be executed before each it() block below. This means each test gets its own instance of the engine. This doesn't matter for now, but in the future when there can be multiple engine instances each can have its own set of filters, etc.

The basic idea is that each test must execute in its own "world."

});

describe('Basic', async () => {
it ('should work', async () => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will later split the contents of this up into multiple tests. This way we know which test failed exactly. Mocha gives nice output.

@mjethani

Copy link
Copy Markdown
Contributor Author

@gorhill I know you're still investigating the testing frameworks. This is just a proof of concept. If you like the idea, I can clean up the patch and mark it ready for review. Any time.

@gorhill

gorhill commented Aug 13, 2021

Copy link
Copy Markdown
Owner

Thanks for this.

I didn't have time to look into this in depth yet but I was indeed planning on using Mocha because the documentation explicitly said they supported ES modules.

My first goal was to be able to record the output of evaluating all of requests.json against uBO's default filter lists with the latest published build of uBO and then use this as reference output to validate no unexpected change with future changes in the code (and also a benchmark test to ensure no unexpected performance regression).

However I am not certain how to do this, because I need to create and use files which are outside the package (requests.json and say snfe.results.json) and keep them around to be used for future tests -- would it be an ok practice to have a directory called, say, data in dist/build and would it be acceptable for the code in the package to access and use the content of that directory?

@mjethani

mjethani commented Aug 13, 2021

Copy link
Copy Markdown
Contributor Author

… would it be an ok practice to have a directory called, say, data in dist/build and would it be acceptable for the code in the package to access and use the content of that directory?

If it's only for the tests and not something that the package needs when it is used by other packages, then it's OK to access files from anywhere within the git repo. If somebody wants to do development on this package, they would have to clone the entire repo anyway.

In the case of requests.json, it's already a Node.js package:

npm i --save-dev mjethani/scaling-palm-tree#15cf1ab

This means it gets installed as a development dependency and the file is available from within it. It will not become part of the archive.

@mjethani

Copy link
Copy Markdown
Contributor Author

In the case of requests.json, it's already a Node.js package

If you want to use it like this, here's the diff for package.json:

@@ -31,6 +31,7 @@
   },
   "devDependencies": {
     "eslint": "^7.32.0",
-    "mocha": "^9.0.3"
+    "mocha": "^9.0.3",
+    "scaling-palm-tree": "github:mjethani/scaling-palm-tree#15cf1ab"
   }
 }

You could fork that repo and link to your own fork if you like. I have used a specific commit hash in the above example so nothing breaks if there's an update.

@gorhill
gorhill marked this pull request as ready for review August 13, 2021 16:51
@mjethani

Copy link
Copy Markdown
Contributor Author

My first goal was to be able to record the output of evaluating all of requests.json against uBO's default filter lists with the latest published build of uBO and then use this as reference output to validate no unexpected change with future changes in the code (and also a benchmark test to ensure no unexpected performance regression).

This sounds like a job for the Cliqz repo, and you could use it as a development dependency too and use a combination of DEBUG=1 and COMPARE=<filename>, where <filename> is the output from a previous stable run with DEBUG=1.

The Mocha test would then simply load the ublock.diff.json file and assert that it has zero entries.

@gorhill

gorhill commented Aug 13, 2021

Copy link
Copy Markdown
Owner

This sounds like a job for the Cliqz repo, and you could use it as a development dependency

I prefer to not have such a dependency -- I want the freedom to modify as I wish that part, for instance consider that I was running a different benchmark and testing more than strictly just matchRequest(), best for me to be able to customize at will without the friction of an external dependency -- I'm fine with depending on a specific hash of https://github.com/mjethani/scaling-palm-tree though.

@mjethani

Copy link
Copy Markdown
Contributor Author

I prefer to not have such a dependency

Sounds good.

It's also fair to say that the original purpose of the code over there is for benchmarking and not really for testing.

The benchmarks will not cover every case but can act as a canary in case something is broken and caught in the benchmarks.

@gorhill
gorhill merged commit 806fe5d into gorhill:master Aug 13, 2021
@mjethani
mjethani deleted the mocha-tests branch August 13, 2021 17:14
@mjethani

Copy link
Copy Markdown
Contributor Author

I have a simple home page here: https://mjethani.github.io/uBlock/

Unfortunately I don't have design skills, and it probably looks awful. But if you like the idea, feel free to pull the site branch. The general idea here is that there should be some kind of simple landing page for the software. This is for regular folks who don't have more than half a second to spare (because they have to look at pictures of cats the rest of the time!).

Maybe we could get a professional designer to take a look and improve it.

@mjethani

Copy link
Copy Markdown
Contributor Author

The general idea here is that there should be some kind of simple landing page for the software.

Oh, I see there's a page here: https://ublockorigin.com/ That's terrific! It's way better. Yeah, I did not know about this. It's the kind of thing I had in mind.

@mjethani

Copy link
Copy Markdown
Contributor Author

My first goal was to be able to record the output of evaluating all of requests.json against uBO's default filter lists with the latest published build of uBO and then use this as reference output to validate no unexpected change with future changes in the code

I think I know how to set this up.

Essentially we have to programmatically convert requests.json into an object that Mocha can consume via the Exports interface.

@gorhill

gorhill commented Aug 14, 2021

Copy link
Copy Markdown
Owner

I already use https://gorhill.github.io/uBlock/ to publish some tests/benchmarks -- in any case, it's not something I feel like spending time on for now, the fan page out there is pretty good and comes up in the first page of results when searching for "ublock", so it's all good.

mneunomne pushed a commit to mneunomne/AdNauseam that referenced this pull request Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants