Support #!/usr/bin/env node shebang and copying original file permissions

[chrisblossom]

Issue description or question

Wallaby removes node shebang #!/usr/bin/env node, does correctly copy file permissions (+x) and tries to instrument CLI only-files, (files with #!/usr/bin/env node) which cause runtime errors.

Example repo: https://github.com/chrisblossom/wallaby-issue-1949

Wallaby.js configuration file

'use strict';

const wallaby = (wallabyConfig) => {
    /**
     * Needed for monorepo
     */
    process.env.NODE_PATH = require('path').join(
        wallabyConfig.localProjectDir,
        '../../node_modules',
    );

    return {
        files: [
            { pattern: 'lib/**/__sandbox__/**/*', instrument: false },
            { pattern: 'lib/**/__sandbox__/**/.*', instrument: false },
            { pattern: 'lib/files/*', instrument: false },
            { pattern: 'lib/files/.*', instrument: false },
            { pattern: '.*', instrument: false },
            { pattern: '*', instrument: false },
            { pattern: '.circleci/*', instrument: false },
            'lib/**/*.js',
            'jest.config.js',
            '.env',
            'lib/**/*.snap',
            '!lib/**/*.test.js',
        ],

        tests: ['lib/**/*.test.js'],

        hints: {
            ignoreCoverage: /ignore coverage/,
        },

        env: {
            type: 'node',
            runner: 'node',
        },

        testFramework: 'jest',

        setup: (setupConfig) => {
            /**
             * link node_modules inside wallaby's temp dir
             *
             * https://github.com/wallabyjs/public/issues/1663#issuecomment-389717074
             */
            const fs = require('fs');
            const path = require('path');
            const realModules = path.join(
                setupConfig.localProjectDir,
                'node_modules',
            );
            const linkedModules = path.join(
                setupConfig.projectCacheDir,
                'node_modules',
            );

            console.log('setupConfig.projectCacheDir', setupConfig.projectCacheDir);
            try {
                fs.symlinkSync(realModules, linkedModules, 'dir');
            } catch (error) {
                if (error.code !== 'EEXIST') {
                    throw error;
                }
            }

            /**
             * Set to project local path so backtrack can correctly resolve modules
             * https://github.com/wallabyjs/public/issues/1552#issuecomment-372002860
             */
            process.chdir(setupConfig.localProjectDir);

            process.env.NODE_ENV = 'test';
            const jestConfig = require('./jest.config');
            setupConfig.testFramework.configure(jestConfig);

            /**
             * https://github.com/wallabyjs/public/issues/1268#issuecomment-323237993
             *
             * reset to expected wallaby process.cwd
             */
            process.chdir(setupConfig.projectCacheDir);
        },
    };
};

module.exports = wallaby;

Code editor or IDE name and version

WebStorm 2018.3.3
Build #WS-183.5153.15, built on December 26, 2018
JRE: 1.8.0_152-release-1343-b26 x86_64
JVM: OpenJDK 64-Bit Server VM by JetBrains s.r.o

OS name and version

macOS 10.14.2

[ArtemGovorov]

Thanks for providing the repo.

Wallaby removes node shebang by design. We have had a few requests from users to do it. However while looking into the issue we have discovered that we are also removing shebang from non instrumented files too (that is a bug). We have fixed it and published the fix in v1.0.639.

I have sent you the pull request with the working config.

[chrisblossom]

@ArtemGovorov Thanks for the PR, but I don't think it helps very much.

What is the reasoning behind removing the node shebang? I can see this only causing issues. Node properly handles the shebang even if another file is requiring the file.

I do not think anyone should have to manually change file permissions via a wallaby config, but instead they should be copied as they were from the source. This looks like a bug to me.

[ArtemGovorov]

What is the reasoning behind removing the node shebang? I can see this only causing issues. Node properly handles the shebang even if another file is requiring the file.

When file is being instrumented, parser needs a valid JavaScript to parse/emit code, however technically code with shebang is not valid JavaScript code, so the parser is not happy. We could probably remove shebang, instrument and attach it back, but there's no point, because:

• if a file is required directly, shebang is not used anyway;
• if a file is an entry point for spawning new process, then there's no point instrumenting it, because Wallaby doesn't support cross process coverage reporting anyway.

I do not think anyone should have to manually change file permissions via a wallaby config, but instead they should be copied as they were from the source.

Not sure if it's going to work (can't try right now), but you may try binary: true flag in addition to instrument: false. This way Wallaby is coping files "as is", with simple file copy operation as opposed to be creating new files.

[chrisblossom]

When file is being instrumented, parser needs a valid JavaScript to parse/emit code, however technically code with shebang is not valid JavaScript code, so the parser is not happy.

I think it is pretty common for parsers to understand / handle the node shebang properly. This is the case for babel's parser and typescript at least.

• if a file is an entry point for spawning new process, then there's no point instrumenting it, because Wallaby doesn't support cross process coverage reporting anyway.

Not sure if it's going to work (can't try right now), but you may try binary: true flag in addition to instrument: false. This way Wallaby is coping files "as is", with simple file copy operation as opposed to be creating new files.

My point is you shouldn't have to configure wallaby for a particular file to get it to work. Adding binary: true is essentially the same thing as setting the permission to 755.

I'm not understanding why jest works without additional configuration, but wallaby doesn't when they are essentially doing the exact same thing.

I understand certain things need additional configuration, I don't think this should be one of them.

[ArtemGovorov]

I'm not understanding why jest works without additional configuration, but wallaby doesn't when they are essentially doing the exact same thing.

Fair enough, we'll look into supporting node shebang and copying file permissions properly.