Esri / esri-leaflet

A lightweight set of tools for working with ArcGIS services in Leaflet. :rocket:
https://developers.arcgis.com/esri-leaflet/
Apache License 2.0
1.61k stars 799 forks source link

Running tests depending on esri-leaflet headlessly #969

Closed colbin8r closed 7 years ago

colbin8r commented 7 years ago

I'm writing a plugin that uses Leaflet and Esri-Leaflet. For my plugin, I have a reasonably large suite of unit tests powered by Mocha, Chai, Sinon, etc. My plugin is structured as a series of ES6 modules.

In my unit tests, I import { FeatureLayerService } from 'esri-leaflet', but this causes the tests to break because esri-leaflet depends on leaflet, which depends upon a global window resulting in a ReferenceError: window is not defined error in a headless environment like Node. I've been able to use pieces of Leaflet itself in my tests thanks to the leaflet-headless package, which sort of hacks over the global window dependency and allows me to import { Map } from 'leaflet-headless' and the like.

My question is: because esri-leaflet has depends on leaflet and not leaflet-headless (and it obviously should), is there any way to allow my unit tests to run, perhaps by:

  1. Forcing esri-leaflet to resolve the leaflet dependency to leaflet-headless (I'm transpiling with Babel and Webpack is my bundler)
  2. Polyfilling/stubbing the window global (I've explored this some using jsdom with no luck)
  3. Ejecting/removing the leaflet dependency from esri-leaflet (not likely since there are obvious dependencies)
  4. Another option I haven't thought of

Code I want to work:

// inside a unit spec
import { Map } from 'leaflet-headless'; // works fine
import { FeatureLayerService } from 'esri-leaflet'; // throws

Resulting errors:

ReferenceError: window is not defined
    at Object.<anonymous> (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/leaflet/dist/leaflet-src
.js:13250:3)
    at Module._compile (module.js:570:32)
    at Module._extensions..js (module.js:579:10)
    at Object.require.extensions.(anonymous function) [as .js] (/mnt/c/dev/Leaflet.Control.NestedLayers/
node_modules/babel-register/lib/node.js:152:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at L$1__default (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/esri-leaflet/dist/esri-leaflet
-debug.js:5:82)
    at Object.<anonymous> (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/esri-leaflet/dist/esri-l
eaflet-debug.js:8:2)
    at Module._compile (module.js:570:32)
    at Module._extensions..js (module.js:579:10)
    at Object.require.extensions.(anonymous function) [as .js] (/mnt/c/dev/Leaflet.Control.NestedLayers/
node_modules/babel-register/lib/node.js:152:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/mnt/c/dev/Leaflet.Control.NestedLayers/src/Leaflet.LayerHierarchy.MapServerP
arser.js:10:1)
    at Module._compile (module.js:570:32)
    at loader (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/babel-register/lib/node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (/mnt/c/dev/Leaflet.Control.NestedLayers/
node_modules/babel-register/lib/node.js:154:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/mnt/c/dev/Leaflet.Control.NestedLayers/test/Leaflet.LayerHierarchy.MapServer
Parser.spec.js:2:1)
    at Module._compile (module.js:570:32)
    at loader (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/babel-register/lib/node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (/mnt/c/dev/Leaflet.Control.NestedLayers/
node_modules/babel-register/lib/node.js:154:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at /mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/lib/mocha.js:230:27
    at Array.forEach (native)
    at Mocha.loadFiles (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/lib/mocha.js:227:14)
    at Mocha.run (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/lib/mocha.js:495:10)
    at loadAndRun (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/bin/_mocha:426:22)
    at rerun (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/bin/_mocha:454:5)
    at /mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/bin/_mocha:462:7
    at StatWatcher.<anonymous> (/mnt/c/dev/Leaflet.Control.NestedLayers/node_modules/mocha/lib/utils.js:
196:9)
    at emitTwo (events.js:106:13)
    at StatWatcher.emit (events.js:191:7)
    at StatWatcher._handle.onchange (fs.js:1501:10)

npm ls | grep leaflet:

├── chai-leaflet@0.0.12
├─┬ esri-leaflet@2.0.8
│ ├── leaflet-virtual-grid@1.0.4
├── leaflet@1.0.3
├─┬ leaflet-headless@0.2.5
│ └─┬ leaflet-image@0.4.0
colbin8r commented 7 years ago

I've realized that it might be better to take this up directly with the Leaflet folks, although there seems to already have been some discussion about it.

jgravois commented 7 years ago

in our own mocha/chai/sinon test suite we pre-load the compiled versions of both leaflet and esri-leaflet via the 'files' parameter in karma.conf.js

files: [
      'node_modules/leaflet/dist/leaflet.css',
      'node_modules/leaflet/dist/leaflet-src.js',
      'dist/esri-leaflet-debug.js',
      // then start loading our actual unit tests
    ],

this allows us to instantiate classes based on compiled UMD class dependencies instead of relying on actual ES6 imports.

  // https://github.com/Esri/esri-leaflet/blob/0da3cf7e17bcd4ef1abd3970bac221b3881eb86a/spec/Services/FeatureLayerSpec.js#L1beforeEach(function () {
    xhr = sinon.useFakeXMLHttpRequest();
    // ...
    service = L.esri.featureLayerService({url: featureServiceUrl});
  });
colbin8r commented 7 years ago

@jgravois Please correct me if I'm wrong or misunderstand, but the only reason this works is that your tests are running in an actual browser environment where window is available and is an actual browser window. Karma is not a headless test runner. I'm running my tests with mocha which uses node, a headless environment where there is no actual browser causing there to be no window object.

Thanks for the answer, but my issue is regarding loading Esri-Leaflet in a headless environment.

Edit: a word

jgravois commented 7 years ago

my issue is regarding loading Esri-Leaflet in a headless environment.

i didn't realize that PhantomJS acts like a real browser. @patrickarlt do you have any ideas/suggestions for @colbin8r?

colbin8r commented 7 years ago

@jgravois I also didn't realize that Karma could run tests in PhantomJS... my mistake! PhantomJS is the closest thing to a headless browser I know of (the goal of the project IIRC). I'll look into using Karma. Sorry for the misunderstanding, @jgravois!

jgravois commented 7 years ago

i'm going to go ahead and close this issue as there's nothing actionable for esri-leaflet at the moment, but i'd be more than happy to discuss the issue further and/or talk about changes in this repo down the line if anyone can recommend something.