derek / yui-benchmark

A toolkit to simplify JavaScript performance testing.
http://derek.io/blog/2013/yui-benchmark/
Other
16 stars 3 forks source link

No mechanism for including dependencies #1

Open lazd opened 10 years ago

lazd commented 10 years ago

I'm trying to include dependencies (other than jQuery), such as the actual code I want to test. So far, I tried:

  1. Using the html option to dump in <script src="somedep.js"></script>
    • The files aren't served anywhere that I can find
  2. Providing the dependencies as additional arguments to yb
    • They are treated as test files, with the first being used and subsequent being ignored

Without starting an additional server and referencing files hosted on it via <script> tags placed in the html option, I can't find a way to do this.

derek commented 10 years ago

The html option was designed for including snippets of HTML into your tests, not really for including scripts. For that latter use-case, take a look at the YUI Gallery example, which demonstrates how any files listed in the assets array will have an HTTP route hooked (src) up to serve the file from the filesystem.

Worth noting that outside of the context of YUI development, the workflow for requesting assets is a little annoying, as you'd have to list all dependencies in your application, then dynamically load in those scripts in a setup function. Within the context of YUI, it's easy because YUI's loader makes it easy.

YUI Benchmark's initial release was solely to ease the pain of perf testing for development on YUI's core, with Gallery development being a secondary priority, and non-YUI development being third. So this is something that could certainly be improved in the future.

FWIW, YUI's Node performance tests are a good example of something that uses the html option for including snippets of HTML to test against.

Leaving this ticket open as a reminder to improve the asset handling logic.

shamasis commented 10 years ago

As of now, the way to get external scripts to work is loading them synchronously using jQuery. A sample test suite would look like

var suite = new PerfSuite({
    name: 'Sync load JS dependencies',
    jquery: true,
    assets: [
        'path/to/file.js'
    ],

    global: {
        setup: function () {
            if (!window.assetsLoaded) {
                $.ajaxSetup({ async: false });
                $.getScript('assets/file.js');
                window.assetsLoaded = true;
            }
        }
    }
});

suite.add({
    name: 'my test',
    async: true,
    fn: function () {
        // you can use globals from your asset file.js in these tests
    }
});
shamasis commented 10 years ago

@derek Do you have the API in mind? The API can be in one of the following ways.

1> Allow a suite property as additional content of <head> of the template HTML. Like suite.headHTML. This may cause referencing issue.

var suite = new PerfSuite({
    assets: ['path/file.js'],
    headHTML: '<script src='file.js'></script>'
});

I don't quite like the headHTML name and that this is a bit unmanaged API - though very flexible.

2> Allow a new suite option called fixtures with keys "application/javascript": [] and "text/css": [] based on mime types. The array refers to file names loaded in assets (makes tests portable.). Internally we map a hash with mime-types to take action on templates with known mime types.

var suite = new PerfSuite({
    assets: ['path/file.js', 'path/file.css'],
    fixtures: {
        'application/javascript': ['file.js'],
        'text/css': ['file.css']
    }
});

This looks a bit nerdy API, but is extensible in the long run and much more managed. The "nerdy-ness" can be reduced if we simply call the keys "javascript" and "css" instead of the mime-types.

PS: The present asset loading mechanism prevents loading two files from different paths with same name.

derek commented 10 years ago

Early in development, there were two HTML options (topHTML and bottomHTML), but that ended up being undesirable and confusing for a few reasons. I think more ideally, YUI Benchmark should use some smarts and generate the <head> HTML for you given a list of files to include, similar to what you have in your second suggestion.

In the second suggestion, I'd prefer to simply list the files to include in an array (assets?), and the application will determine how to include them. The mime type could be fetched from the filesystem and mapped back to a <script type='application/javascript' src='{{src}}'></script> or <link href='{{src}}' rel='stylesheet' type='text/css'> template to use. In the future, support for additional filetypes could be addressed, but it seems premature at this point.

derek commented 10 years ago

Additional thought... I like the idea of including those files in the existing assets option, as they are assets to be included in the application, they just have a slightly different behavior. For example, the current default behavior for assets is to simply map a path from the HTTP server to the file system so they can be fetched via YUI's loader. In this new feature, they need to also be included into the HTML via a <script> tag in the <head>. This makes me lean towards assets being an array of objects and strings. Strings representing a short-hand for the default behavior (TBD), and an object for further customization.

Short-hand

var suite = new PerfSuite({
    assets: ['path/to/file.js']
});

Long-hand (exact option names TBD)

var suite = new PerfSuite({
    assets: [
        {
            filesystemPath: 'path/to/file.js'
            servingPath: 'different/path/to/file.js'
            mimeType: 'application/javascript'
            includeInHead: true
            templateHTML: '<script type='application/javascript' src='{{src}}'></script>'
        }
    ]
});
shamasis commented 10 years ago

This API even solves the issue with assets not being able to load same file name with different paths. :-) Can you explain includeInHead? Is it a way to specify where the templateHTML be written? If false, does it write to body start or body end?

By this long-form method, user has to specify the templateHTML for all assets. Will it not be too much for users to write if one includes a number of assets?

derek commented 10 years ago

Can you explain includeInHead? Is it a way to specify where the templateHTML be written? If false, does it write to body start or body end?

I believe there would be two areas where you would want to include assets, in the <head> and <body>. JS and CSS in the head, and maybe some HTML for the body. So that's the reasoning for it being specific to the head in the option name. However, it doesn't make sense for the same asset to be included in the head and body, so it probably makes more sense for it to be a single option, such as location: {'head'/'body'/false}. false representing the current behavior of not including it in the HTML (serving via HTTP only).

If asset.templateHTML is not a string (e.g. false), it will be automatically determined (e.g. js file becomes <script>)

Oh, also templateHTML should really just be template, since it could be any kind of string (e.g.{{mustache}} templates).

By this long-form method, user has to specify the templateHTML for all assets. Will it not be too much for users to write if one includes a number of assets?

It should be possible to mix long-form and short-form in the assets option, as well as leaving out any options that should be left as default. filesystemPath being the only option that is required.

Sporran commented 9 years ago

To my mind a serious drawback to an otherwise very interesting tool. Shamasis' suggestion to use jQuery goes some way towards a solution, but doesn't seem to address the issue of establishing global (module namespacing) variables à la node.js:

    var vows    = require('vows')
      , assert  = require('assert')
      , teoria  = require('teoria')
      , piu     = require('../index');
    :

Before noticing the existence of this issue (why no mention in the readme?), I had already placed a related question on stackoverflow. I have neither the knowledge nor skills to resolve this, but feel that without a fix, yui-benchmark will fail to be adopted.