Fix issue with multipass and add tests (#1)

* Fix issue with multipass and add tests * Dont insert newline at end of file * Use checkout v2 * Fix lint error
innocuous-tech · Mar 8, 2020 · aaebe39 · aaebe39
1 parent a600594
commit aaebe39
Show file tree

Hide file tree

Showing 8 changed files with 133 additions and 8 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -7,7 +7,7 @@ indent_size = 4
 end_of_line = lf
 charset = utf-8
 trim_trailing_whitespace = true
-insert_final_newline = true
+insert_final_newline = false
 
 [Makefile]
 indent_style = tab

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -11,7 +11,7 @@ jobs:
         node-version: [8.x, 10.x, 12.x]
 
     steps:
-    - uses: actions/checkout@v1
+    - uses: actions/checkout@v2
     - name: Use Node.js ${{ matrix.node-version }}
       uses: actions/setup-node@v1
       with:

diff --git a/lib/svgo.js b/lib/svgo.js
@@ -23,6 +23,8 @@ var SVGO = function(config) {
 
 SVGO.prototype.optimize = function(svgstr, info) {
     info = info || {};
+    info.multipassCount = 0;
+
     return new Promise((resolve, reject) => {
         if (this.config.error) {
             reject(this.config.error);
@@ -39,8 +41,8 @@ SVGO.prototype.optimize = function(svgstr, info) {
                     return;
                 }
 
-                info.multipassCount = counter;
-                if (++counter < maxPassCount && svgjs.data.length < prevResultSize) {
+                var numberOfPasses = info.multipassCount = ++counter;
+                if (numberOfPasses < maxPassCount && svgjs.data.length < prevResultSize) {
                     prevResultSize = svgjs.data.length;
                     this._optimizeOnce(svgjs.data, info, optimizeOnceCallback);
                 } else {

diff --git a/test/coa/_index.js b/test/coa/_index.js
@@ -1,12 +1,20 @@
 'use strict';
 
 const fs = require('fs'),
+    yaml = require('js-yaml'),
     svgo = require(process.env.COVERAGE ?
         '../../lib-cov/svgo/coa.js' :
         '../../lib/svgo/coa.js').api,
+    defaults = Object.assign({}, yaml.safeLoad(fs.readFileSync(__dirname + '/../../.svgo.yml', 'utf8'))),
     path = require('path'),
     svgPath = path.resolve(__dirname, 'test.svg'),
     svgFolderPath = path.resolve(__dirname, 'testSvg'),
+    prefixIdsFolderPath = path.resolve(__dirname, 'testPrefixIds'),
+    prefixIdsSvgInPath  = path.resolve(prefixIdsFolderPath, 'in.svg'),
+    mpDirPath     = path.resolve(__dirname, 'testMultipass'),
+    mpSvgInPath   = path.resolve(mpDirPath, 'in.svg'),
+    mpSvgExpPath  = path.resolve(mpDirPath, 'out.svg'),
+    mpSvgExp      = fs.readFileSync(mpSvgExpPath, 'utf8'),
     svgFolderPathRecursively = path.resolve(__dirname, 'testSvgRecursively'),
     svgFiles = [path.resolve(__dirname, 'testSvg/test.svg'), path.resolve(__dirname, 'testSvg/test.1.svg')],
     tempFolder = 'temp',
@@ -157,6 +165,79 @@ describe('coa', function() {
         }
     });
 
+    it('should pass the filename to the prefixIds plugin', function(done) {
+        svgo({
+            input:     prefixIdsSvgInPath,
+            output:    'temp.svg',
+            quiet:     true,
+            multipass: false,
+            disable: defaults.plugins, // disable all plugins except ...
+            enable: [ 'prefixIds' ]    // ... prefixIds
+        }).then(function() {
+            const svgOut = fs.readFileSync('temp.svg', 'utf8');
+
+            done(/in_svg__/.test(svgOut) ? null : 'filename isn\'t passed to prefixIds plugin.');
+            fse.removeSync('temp.svg');
+        }, error => done(error));
+    });
+
+    describe('multipass', function() {
+        it('should optimize using multiple passes with multipass enabled', function(done) {
+            svgo({
+                input:     mpSvgInPath,
+                output:    'temp.svg',
+                quiet:     true,
+                multipass: true
+            }).then(function() {
+                const mpSvgOut = fs.readFileSync('temp.svg', 'utf8');
+                done(mpSvgOut === mpSvgExp ? null : 'Multipass wasn\'t properly used.');
+                fse.removeSync('temp.svg');
+            }, error => done(error));
+        });
+
+        it('should allow prefixId plugin to detect subsequent passes with multipass enabled', function(done) {
+            svgo({
+                input:     mpSvgInPath,
+                output:    'temp.svg',
+                quiet:     true,
+                multipass: true,
+                disable: defaults.plugins, // disable all plugins except ...
+                enable: [ 'prefixIds' ]    // ... prefixIds
+            }).then(function() {
+                const mpSvgOut = fs.readFileSync('temp.svg', 'utf8');
+
+                done(!/in_svg__in_svg__/.test(mpSvgOut) ? null : 'prefixIds plugin doesn\'t detect subsequent passes with multipass enabled.');
+
+                // https://github.com/svg/svgo/issues/659
+                // https://github.com/svg/svgo/issues/1133
+                fse.removeSync('temp.svg');
+            }, error => done(error));
+        });
+
+        it('should allow addAttributesToSVGElement plugin to correctly handle subsequent passes with multipass enabled', function(done) {
+            svgo({
+                input:     mpSvgInPath,
+                output:    'temp.svg',
+                quiet:     true,
+                multipass: true,
+                config: `{
+                    "plugins": [{ "addAttributesToSVGElement": {
+                        "attribute": "aria-hidden=\\"true\\""
+                    } }]
+                }`
+            }).then(function() {
+                const mpSvgOut = fs.readFileSync('temp.svg', 'utf8');
+
+                done(!/aria-hidden="true" aria-hidden='true'/.test(mpSvgOut) ? null : 'addAttributesToSVGElement plugin doesn\'t correctly handle subsequent passes with multipass enabled.');
+
+                // https://github.com/svg/svgo/issues/659
+                // https://github.com/svg/svgo/issues/1133
+                fse.removeSync('temp.svg');
+            }, error => done(error));
+        });
+    });
+
+
     describe('stdout', function() {
         it('should show file content when no output set', function(done) {
             replaceConsoleLog();

diff --git a/test/coa/testMultipass/in.svg b/test/coa/testMultipass/in.svg
diff --git a/test/coa/testMultipass/out.svg b/test/coa/testMultipass/out.svg
diff --git a/test/coa/testPrefixIds/in.svg b/test/coa/testPrefixIds/in.svg
diff --git a/test/svgo/_index.js b/test/svgo/_index.js
@@ -9,9 +9,7 @@ var FS = require('fs'),
                    '../../lib/svgo');
 
 describe('indentation', function() {
-
     it('should create indent with 2 spaces', function(done) {
-
         var filepath = PATH.resolve(__dirname, './test.svg'),
             svgo;
 
@@ -34,13 +32,39 @@ describe('indentation', function() {
                 normalize(result.data).should.be.equal(should);
                 done();
             });
-
         });
-
     });
+});
+
+describe('invocation', function() {
+    it('should optimize without an info object', function(done) {
+        var filepath = PATH.resolve(__dirname, './test.svg'),
+            svgo;
 
+        FS.readFile(filepath, 'utf8', function(err, data) {
+            if (err) {
+                throw err;
+            }
+
+            var splitted = normalize(data).split(/\s*@@@\s*/),
+                orig     = splitted[0],
+                should   = splitted[1];
+
+            svgo = new SVGO({
+                full    : true,
+                plugins : [],
+                js2svg  : { pretty: true, indent: 2 }
+            });
+
+            svgo.optimize(orig, undefined).then(function(result) {
+                normalize(result.data).should.be.equal(should);
+                done();
+            });
+        });
+    });
 });
 
+
 function normalize(file) {
     return file.trim().replace(regEOL, '\n');
 }