Instructions on how to use?

andrefox333 commented 6 years ago

Not really sure how to use this plugin with Jest. I installed it and hoping the following error would go away...

TypeError: window.URL.createObjectURL is not a function

But the error still exist...

mwarren2 commented 6 years ago

Can we please have some documentation than can help people start testing mapbox their own apps?

At the moment I have found nothing anywhere in mapbox that helps with testing for apps that use mapbox. I spent nearly a month struggling to get something together that would test a simple routine that used a Map, in part because there was nothing here to help me get started, which was a little frustrating.

I presume that if I could work out how to use this mock it would be very useful - especially since instantiating a real Map with jsdom seems to take quite a long time, and like everyone else I like fast unit tests.

So, please - may we have a little documentation ?

haysclark commented 6 years ago

Not docs, but some discussions on this subject:

mwarren2 commented 6 years ago

Not docs, but some discussions on this subject:

mapbox/mapbox-gl-js#5026

jsdom/jsdom#1721

Thanks @haysclark , that was helpful.

I have here laid out what I had to do to get a simple Map instantiated for a test: it may save other people time.

I use jest, because it seems to give more useful error messages than mocha. But some of this applies to mocha as well.

Thanks to a pointer by @fc in mapbox/mapbox-gl-js#5026 I can finally use mapbox-gl-js-mock in my tests: jest.mock("mapbox-gl", () => require("mapbox-gl-js-mock"));

His PR request was merged in the current release of mapbox-gl (0.51.0) for the error:

" SecurityError: localStorage is not available for opaque origins"

But mapbox-gl-js-mock still depends on mapbox-gl 0.44.1 and so we still get the error. Therefore until this is dealt with you have to hack it by editing: node_modules/mapbox-gl-js-mock/node_modules/mapbox-gl/src/util/window.js

Go to line 19 where you will see this:

    const { window } = new jsdom.JSDOM('', {     
        // Send jsdom console output to the node console object.
        virtualConsole: new jsdom.VirtualConsole().sendTo(console)
    });

Change it to the following by adding the line url: 'http://localhost':

    const { window } = new jsdom.JSDOM('', {     
        url: 'http://localhost',                 
        // Send jsdom console output to the node console object.
        virtualConsole: new jsdom.VirtualConsole().sendTo(console)
    });

So finally I can instantiate a Map suitable for testing !

import mapboxgl from 'mapbox-gl';
jest.mock("mapbox-gl", () => require("mapbox-gl-js-mock"));

import jsdom from 'jsdom';
const { JSDOM } = jsdom;
const dom = new JSDOM(`<!DOCTYPE html><div id"myMap"></div>`);

let mapDiv = dom.window.document.querySelector("div");
let map = new mapboxgl.Map({container: mapDiv});

However, for those who are starting from scratch and are using ES6, you will also need to sort babel out or you will get your favourite error "unexpected token import", among others.

In package.json in my dependencies I have only "@babel/runtime": "^7.0.0-beta.49" and here are my devDependencies:

"devDependencies": {
    "@babel/cli": "^7.1.2",
    "@babel/core": "^7.1.2",
    "@babel/plugin-transform-flow-strip-types": "^7.0.0",
    "@babel/preset-env": "^7.1.0",
    "@babel/preset-flow": "^7.0.0",
    "@babel/register": "^7.0.0",
    "babel-core": "^7.0.0-bridge.0",
    "babel-jest": "^23.6.0",
    "canvas": "^2.0.1",
    "chai": "^4.2.0",
    "chromedriver": "^2.42.0",
    "elementtree": "^0.1.7",
    "faker": "^4.1.0",
    "gl": "^4.1.1",
    "jest": "^23.6.0",
    "jest-cli": "^23.6.0",
    "jsdom": "^12.2.0",
    "mapbox-gl-js-mock": "^1.0.2",
    "mocha": "^5.2.0",
    "npm-run-all": "^4.1.3",
    "plist": "^2.1.0",
    "regenerator-runtime": "^0.12.1",
    "selenium-webdriver": "^3.6.0",
    "sinon": "^6.3.4",
    "sinon-chai": "^3.2.0"
},

I found I needed all those babel devDependencies including the babel-core with the bridge. Besides the usual "@babel/preset-env" you also need "@babel/preset-flow" because mapbox-gl includes flow code, which will have to be stripped.

I don't have a .babelrc file, instead I have a babel.config.js file which looks like this:

module.exports = function(api) {
    // The API exposes the following:

    // Cache the returned value forever and don't call this function again.
    api.cache(true);

    // Cached based on the value of some function. If this function returns a value different from
    // a previously-encountered value, the plugins will re-evaluate.
    // var env = api.cache(() => process.env.NODE_ENV);
    //
    // If testing for a specific env, we recommend specifics to avoid instantiating a plugin for
    // any possible NODE_ENV value that might come up during plugin execution.
    // var isProd = api.cache(() => process.env.NODE_ENV === "production");
    //
    // .cache(fn) will perform a linear search though instances to find the matching plugin based
    // based on previous instantiated plugins. If you want to recreate the plugin and discard the
    // previous instance whenever something changes, you may use:
    // var isProd = api.cache.invalidate(() => process.env.NODE_ENV === "production");
    //
    // Note, we also expose the following more-verbose versions of the above examples:
    //api.cache.forever(); // api.cache(true)
    //api.cache.never();   // api.cache(false)
    //api.cache.using(fn); // api.cache(fn)

    //Flow enables type checking, so you need babel plugin strip-types to deal with
    //stuff like "import type {window} from 'whatever'" which appears in a lot of Mapbox code
    return {
        presets: [
            "@babel/preset-env",
            "@babel/preset-flow"
        ],
        plugins: [
            "@babel/plugin-transform-flow-strip-types"
        ]
    };
};

Then you will need a jest.config.js file with at least this content:

const {defaults} = require('jest-config');

module.exports = {
    verbose: true,
    "transformIgnorePatterns": [
        "/node_modules\\/(?!(mapbox-gl))/",
    ],
    "setupFiles": [
        "./jest.stubs.js",
    ]
};

You need the transformIgnorePatterns key because by default babel will not transpile anything in node_modules, otherwise your test would take a week to run. But we want mapbox-gl code to be transpiled so we tell it not to ignore mapbox-gl.

Incidentally the verbose key doesn't seem to have any effect whatever I do, but that's not so important.

I actually have nothing in my jest.stubs.js file but you may need it one day.

fc commented 6 years ago

Just a footnote that my original PR was incorrect and the actual fix landed in this PR: https://github.com/mapbox/mapbox-gl-js/pull/7516/files

v0.51.0 was released but mapbox-gl-js-mock still depends on 0.44.1 when you do an install (sub 1.0.0 versions don't pull in newer versions even when there is a carrot...).

See here: https://github.com/mapbox/mapbox-gl-js-mock/blob/master/package.json#L30

Anyway, so I think the right fix will be to update the mapbox-gl dependency for mapbox-gl-js-mock to 0.51.0 (should be able to easily test this by creating a fork with the change...).

mwarren2 commented 6 years ago

@fc thanks for the correction. I'll edit to reflect this. This mock definitely isn't getting enough love.

mwarren2 commented 6 years ago

@fc I've forked the mock as you wisely suggested, and it crashes with

TypeError: Evented is not a constructor at new Map (node_modules/mapbox-gl-js-mock/classes/map.js:53:19)

This is because /src/util/evented.js from mapbox-gl originally exported just one object with: module.exports = Evented;

now it exports three separate classes, one of which is called Evented. If you fix that, it fails again with a similar problem on another class.

In other words things have changed so much that it's hopeless.

I'll open another issue asking for the mapbox-gl dependency to be updated (if there isn't one already open) and keep my fingers crossed.

fc commented 6 years ago

@mwarren2 created PR, #34

fc commented 6 years ago

yarn/npm will let you install from github, so you could, in the short term, run yarn add fc/mapbox-gl-js-mock#upgrade-mapbox to use the version from the PR.

mwarren2 commented 6 years ago

@fc I'd just like to say thanks for your help !

allthesignals commented 5 years ago

@mwarren2 out of curiosity, what approach do you take with testing apps that use mapbox-gl/draw? I have a lot of trouble simulating clicks to test functionality. I end up stubbing in a lot of my own application code to get around this, which misses a lot of bugs.

You may be the only person I've seen on here who has raised issue with testing. I run into a lot of problems with testing functionality that depends on these maps. Even worse, mapbox-gl-js is deprecating the private-ish map.fire method, which is important for testing behavior, even for mapbox-gl addons.

mwarren2 commented 5 years ago

@allthesignals

I'm afraid I only got modest results so far testing Mapbox code. This is despite spending a lot of time trying to get tests to work, because decent tests could dramatically speed up my work. However I did get somewhere, and it has had a good knock-on effect.

The only way (don't hold you breath) I could find to test a particular method with a click was to say (simulated code):

if(signed-in-to-meteor){
    if(image.clicked()){
        //call the method that moves that image smoothly towards that other object, 
        //so at least I can see it working on desktop without having to build 
        //and install the app every time on my iphone
    }
}

It works, but I don't have any method more sophisticated than that.

If you're interested I have found a way finally of instantiating a real Map in a unit test using Jest (it involves a couple of smallish hacks). And this enabled me to solve a couple of long-term problems that were slowing me down.

The problem is that mapbox simply hasn't got its act together at all as far as helping people run tests is concerned.

1) This mapbox-gl-js-mock which they started is already completely out of date and they don't seem to have any intention of continuing with it.

2) Mapbox doesn't use ES6, so they have no need for babel. So for the rest of us their tests aren't useable, even if you poke around and work out what's going on in them.

3) They use Tap, whereas many of us use Mocha or Jest.

allthesignals commented 5 years ago

The only way (don't hold you breath) I could find to test a particular method with a click was to say (simulated code):
if(signed-in-to-meteor){
    if(image.clicked()){
        //call the method that moves that image smoothly towards that other object, 
        //so at least I can see it working on desktop without having to build 
        //and install the app every time on my iphone
    }
}
It works, but I don't have any method more sophisticated than that.

Thank you, @mwarren2! I'm not sure I quite follow, but it sounds like you've found a workaround here for dealing with simulated clicks. It sounds like you are triggering the callback events directly, which makes sense - that's how I've been able to handle most of it.

The problem I run into is that there is a large degree of application-specific code when it comes to configuring things like mapbox-gl-draw. For example, we wire up a lot of draw's callback events to different component behaviors. These depend on certain assumptions about how draw's event system will behave in response to certain actions (clicks!). Unfortunately, triggering callbacks directly has missed a lot of problems. Once I totally missed a memory leak that deteriorated the app after subsequent invocations of draw because I was stubbing things in to trigger callbacks directly (always sanity check things).

When it comes to mapbox's own testing, mapbox-gl-draw has a lot of complex examples using simulated clicks. I have tried to reverse-engineer parts of this, but it would help for a full explanation, maybe a blog post, detailing best practices and pitfalls. These tests also rightly depend on stubbing in a lot of simplified dummy data so that reasoning about screen coordinates, unprojected geographic coordinates, and so on, is a lot easier.

The issue here, however, is that map.fire is (sorta) deprecated! I would lobby against its deprecation or for promoting it as a public API.

Another helpful source-code-level pattern I've found is embracing dependency injection, which works well for "bindings" addons like this. You get access to the dependency's instance, of course, so you can check basic things (like whether layers were added). Spies work well here too.

For the apps we build, though, the map is the main event, and it's usually couched in a lot of complementary routing logic, components, services, etc, but all of these interact with that map a lot. So we see a ton of regressions because I just can't test the darn things.

I'm going to spend a few hours with a sandbox app and set up a few tests based on the patterns I see here.

This mapbox-gl-js-mock which they started is already completely out of date and they don't seem to have any intention of continuing with it.

Mapbox doesn't use ES6, so they have no need for babel. So for the rest of us their tests aren't useable, even if you poke around and work out what's going on in them.

They use Tap, whereas many of us use Mocha or Jest.

I've heard of a lot of people creating their own "noop"-ish blackhole version of mapbox-gl-js (it seems like you just forked it). Maybe when you find holes in the API, they're filled in on an as-needed basis? I went down this path once but ran into some weird problems.

Anyways, I will keep working on this (considering that all of our apps are map-based and need testing), and report anything useful I've found!

mwarren2 commented 5 years ago

@allthesignals You're way ahead of what I've done.

Yes, I included some testing code directly in my app in order to test the clicks. Which is not ideal, but got the job done for now.

Just for the record I'll write later exactly what I did to get a Map directly under test. You have to make a couple of small changes in mapbox-gl-js code that will get written over when you update to a new version, so essentially yes, it's a fork.

I think probably deprecating map.fire can be fought against once they realise its importance for testing.

allthesignals commented 5 years ago

@mwarren2

After a little research and looking into mapbox-gl-js's own test suite, I've found it works really well if I stub in a specific method:

    await waitUntil(() => map.queryRenderedFeatures().length, { timeout: 15000 });

    const allRenderedFeatures = map.queryRenderedFeatures();

    // stub in and return all the rendered features
    Sinon.stub(map, 'queryRenderedFeatures').callsFake((point, options) => { // eslint-disable-line
      return allRenderedFeatures;
    });

    await click(map.getCanvas());

This way I get control over what is returned from a click and I don't have to worry about un-projecting things. This approach depends on Sinon.

mwarren2 commented 5 years ago

@allthesignals This is what I do to instantiate a Map for a test (using Jest), is not such a big hack really. However these steps will be overwritten every time you update mapbox-gl. However if you need babel, there's a bit of work to do with devDependencies.

There's a file called util.js in a folder called /test in mapbox-gl on github, which doesn't get installed with the other content in node_modules. It contains a method called createMap(), which is what we are looking for.
Copy all the code in this file from mapbox-gl on github and place it in node_modules/mapbox-gl/test/utils.js
esm, which is a dependency of mapbox-js, for some reason gives a strange error. In order to track down the error I installed an un-minified version of esm.js and the error disappeared. So you need to take /node_modules/esm/esm.js, de-minify it on one of the many de-minifying sites, rename esm.js to esmORIGINAL.js, and save the de-minified version as esm.js

Then if you are using es6 or above you want babel so you will need to do everything in my comment above from this phrase onwards: However, for those who are starting from scratch and are using ES6, you will also need to sort babel out or you will get your favourite error "unexpected token import", among others. (ignore everything before this phrase because it involves mapbox-gl-js-mock, which is deprecated)

The actual test looks like this:

import sinon from 'sinon';
import { createMap } from '../node_modules/mapbox-gl/test/util';

/**
 * The mapbox-gl test/util createMap() function needs a Tap instance because it calls a 
*  tap.stub() method.
 * We use sinon instead because it has a method - sinon.stub() - with the same parameters.
 * And it works... !
 * You can also call map.remove(); at the end if you think you are running into memory 
 * problems.
 */
const map = createMap(sinon);

The first time you run it, it takes a long time. But then a cache kicks in and from then on it's pretty quick.

mapbox / mapbox-gl-js-mock

Instructions on how to use? #27