From 403e4cf68e6ff060933630c2878532396a844376 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Tue, 25 Apr 2017 13:02:14 -0400 Subject: [PATCH] Spec improvements --- lib/obj2gltf.js | 25 ++++++++++++--------- specs/lib/loadObjSpec.js | 8 +++---- specs/lib/obj2gltfSpec.js | 46 ++++++++++++++++++++------------------- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lib/obj2gltf.js b/lib/obj2gltf.js index 3d4386b..9255a06 100644 --- a/lib/obj2gltf.js +++ b/lib/obj2gltf.js @@ -11,7 +11,6 @@ var loadObj = require('./loadObj'); var writeUris = require('./writeUris'); var fsExtraOutputJson = Promise.promisify(fsExtra.outputJson); -var fsExtraRemove = Promise.promisify(fsExtra.remove); var defaultValue = Cesium.defaultValue; var defined = Cesium.defined; @@ -85,7 +84,7 @@ function obj2gltf(objPath, gltfPath, options) { } gltfPath = path.join(path.dirname(gltfPath), modelName + extension); - var resourcesDirectory = options.bypassPipeline ? path.dirname(gltfPath) : getTempDirectory(); + var resourcesDirectory = options.bypassPipeline ? path.dirname(gltfPath) : obj2gltf._getTempDirectory(); var aoOptions = ao ? {} : undefined; var kmcOptions = kmc ? {} : undefined; @@ -121,23 +120,20 @@ function obj2gltf(objPath, gltfPath, options) { return GltfPipeline.processJSONToDisk(gltf, gltfPath, pipelineOptions); } }) - .then(function() { + .finally(function() { return cleanup(resourcesDirectory, options); }); } function cleanup(resourcesDirectory, options) { if (!options.bypassPipeline && options.separate) { - return fsExtraRemove(resourcesDirectory); + fsExtra.remove(resourcesDirectory, function () { + // Don't fail simply because we couldn't + // clean up the temporary files. + }); } } -function getTempDirectory() { - var tempDirectory = os.tmpdir(); - var randomId = uuid.v4(); - return path.join(tempDirectory, randomId); -} - /** * Default values that will be used when calling obj2gltf(options) unless specified in the options object. */ @@ -230,6 +226,15 @@ obj2gltf.defaults = { */ obj2gltf._outputJson = fsExtraOutputJson; +/** + * Exposed for testing + * + * @private + */ +obj2gltf._getTempDirectory = function () { + return path.join(os.tmpdir(), uuid()); +}; + /** * A callback function that logs messages. * @callback Logger diff --git a/specs/lib/loadObjSpec.js b/specs/lib/loadObjSpec.js index 3375e64..d41124c 100644 --- a/specs/lib/loadObjSpec.js +++ b/specs/lib/loadObjSpec.js @@ -62,6 +62,10 @@ function getImagePath(objPath, relativePath) { var defaultOptions = obj2gltf.defaults; describe('loadObj', function() { + beforeEach(function() { + spyOn(console, 'log'); + }); + it('loads obj with positions, normals, and uvs', function(done) { expect(loadObj(objUrl, defaultOptions) .then(function(data) { @@ -270,7 +274,6 @@ describe('loadObj', function() { }); it('loads obj with missing mtllib', function(done) { - spyOn(console, 'log'); expect(loadObj(objMissingMtllibUrl, defaultOptions) .then(function(data) { expect(data.materials).toEqual({}); @@ -288,8 +291,6 @@ describe('loadObj', function() { }); it('does not load resources outside of the obj directory when secure is true', function(done) { - spyOn(console, 'log'); - var options = clone(defaultOptions); options.secure = true; @@ -314,7 +315,6 @@ describe('loadObj', function() { }); it('loads obj with missing texture', function(done) { - spyOn(console, 'log'); expect(loadObj(objMissingTextureUrl, defaultOptions) .then(function(data) { var imagePath = getImagePath(objMissingTextureUrl, 'cesium.png'); diff --git a/specs/lib/obj2gltfSpec.js b/specs/lib/obj2gltfSpec.js index eda21f7..3cac5f9 100644 --- a/specs/lib/obj2gltfSpec.js +++ b/specs/lib/obj2gltfSpec.js @@ -1,6 +1,9 @@ 'use strict'; +var fsExtra = require('fs-extra'); var GltfPipeline = require('gltf-pipeline').Pipeline; +var os = require('os'); var path = require('path'); +var Promise = require('bluebird'); var obj2gltf = require('../../lib/obj2gltf'); var writeUris = require('../../lib/writeUris'); @@ -10,28 +13,33 @@ var glbPath = 'specs/data/box-textured/box-textured.glb'; var objPathNonExistent = 'specs/data/non-existent.obj'; describe('obj2gltf', function() { + var tempDirectory; + + beforeAll(function() { + expect(obj2gltf._getTempDirectory()).toContain(os.tmpdir()); + tempDirectory = path.join(os.tmpdir(), 'testPath'); + spyOn(obj2gltf, '_getTempDirectory').and.returnValue(tempDirectory); + spyOn(obj2gltf, '_outputJson'); + spyOn(writeUris, '_outputFile'); + spyOn(fsExtra, 'remove'); + }); + + beforeEach(function() { + spyOn(GltfPipeline, 'processJSONToDisk').and.returnValue(Promise.resolve()); + }); + it('converts an obj to gltf', function(done) { - var spy = spyOn(GltfPipeline, 'processJSONToDisk'); expect(obj2gltf(objPath, gltfPath) .then(function() { - var args = spy.calls.first().args; + var args = GltfPipeline.processJSONToDisk.calls.first().args; var gltf = args[0]; var outputPath = args[1]; + var options = args[2]; expect(path.normalize(outputPath)).toEqual(path.normalize(gltfPath)); expect(gltf).toBeDefined(); expect(gltf.images.cesium).toBeDefined(); - }), done).toResolve(); - }); - - it('uses default gltf-pipeline options', function(done) { - var spy = spyOn(GltfPipeline, 'processJSONToDisk'); - expect(obj2gltf(objPath, gltfPath) - .then(function() { - var args = spy.calls.first().args; - var options = args[2]; - expect(options.basePath).toBeDefined(); - delete options.basePath; // This will be a random temp directory expect(options).toEqual({ + basePath : tempDirectory, createDirectory : false, binary : false, embed : true, @@ -50,8 +58,6 @@ describe('obj2gltf', function() { }); it('sets options', function(done) { - var spy = spyOn(GltfPipeline, 'processJSONToDisk'); - spyOn(writeUris, '_outputFile'); var textureCompressionOptions = { format : 'dxt1', quality : 10 @@ -74,11 +80,10 @@ describe('obj2gltf', function() { expect(obj2gltf(objPath, gltfPath, options) .then(function() { - var args = spy.calls.first().args; + var args = GltfPipeline.processJSONToDisk.calls.first().args; var options = args[2]; - expect(options.basePath).toBeDefined(); - delete options.basePath; // This will be a random temp directory expect(options).toEqual({ + basePath : tempDirectory, createDirectory : false, binary : true, embed : false, @@ -98,18 +103,15 @@ describe('obj2gltf', function() { }); it('saves as binary if gltfPath has a .glb extension', function(done) { - var spy = spyOn(GltfPipeline, 'processJSONToDisk'); expect(obj2gltf(objPath, glbPath) .then(function() { - var args = spy.calls.first().args; + var args = GltfPipeline.processJSONToDisk.calls.first().args; var options = args[2]; expect(options.binary).toBe(true); }), done).toResolve(); }); it('bypassPipeline flag bypasses gltf-pipeline', function(done) { - spyOn(obj2gltf, '_outputJson'); - spyOn(GltfPipeline, 'processJSONToDisk'); var options = { bypassPipeline : true };