Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Split the codebase #378

Closed
jzaefferer opened this Issue · 12 comments

6 participants

@jzaefferer
Owner

Up for discussion: Currently the QUnit code base is just a single js file (with companion css). That makes it possible to include QUnit as a submodule, from tag or latest, no need to build anything. It also makes maintenance kind of a pain, as we're now dealing with 2k+ LoC in a single file. Finding things isn't easy. A good bunch of code lends itself to be moved to separate modules, like QUnit.eqiv or QUnit.jsDump. That would make cleanup of the remaining pieces a lot easier.

When working on QUnit, we could use grunt watch to trigger a build for every change, similar how development on jQuery Core works.

As for usage of the QUnit "binary":

  • jQuery Core uses QUnit as a submodule: That could be replaced by npm install qunitjs and referencing node_modules/qunit/ directly or copying the files to whereever they're supposed to live. npm modules can contain "binaries", so whatever the build outputs can be published through npm, without commiting it to Git.
  • jQuery UI and jQuery Mobile include QUnit as copies in its externals folder. That is updated manually by copying over the files, so that shouldn't be a problem at all.

What else is there that we need to take into account?

@jzaefferer
Owner

This would also make #364 much less painful to resolve.

@rodneyrehm

+1 for splitting the codebase. I've been trying to get a grip on QUnit's source for some time now. It's a pain. A single file should only concern itself with a single topic - and there are a bunch of them:

  • core (initialization, state, event bus, …)
  • module, test, … ("organizational infrastructure")
  • assertions ()
  • output visualization ("rendering the DOM", not required if you're running JS-only, btw.)
  • diff algorithm
  • diff visualization ("rendering", not required for JS-only)
  • exception handling (btw. is TraceKit something?)
  • probably more…

that would also allow throwing the addons into the central source directory.

I'm fond of RequireJS and UMD. Since we're dealing with a test suite (that is not served to millions of people) I don't see the point in building a single optimized file.

@JamesMGreene
Collaborator

I think splitting this up is a great idea. Like @rodneyrehm, I also love RequireJS and UMD. However, I think it is important to build a single optimized "qunit.js" file for all those consumers who have not yet adopted RequireJS/AMD as their standard (which is also why we would use a form of UMD instead of AMD).

As far as UMD wrappers go, I've been the most impressed by what @jdalton did with Lo-Dash and I've been modeling my own UMD wrappers after that.

@dmethvin
Owner

Having just looked at adding a feature to QUnit I am now on the side of splitting this up and having a build process. It would be nice to have the markup in a template that got compiled to JavaScript so there wouldn't be so much string slinging throughout.

@JamesMGreene
Collaborator

I'd love to see the HTML pretty much sliced out all together (as feasible) so that QUnit could support multiple reporters types (like Mocha does) with minimal DOM overhead.

@Krinkle
Collaborator

Reminder: As part of splitting it up in a more modular code base, also rename the qunit/ directory to src/.

@jdalton

fwiw I get QUnit working in Rhino, Rhino --require, Ringo, Narwhal, Node, & browsers using:

  /** Use a single "load" function */
  var load = typeof require == 'function' ? require : window.load;

  /** The unit testing framework */
  var QUnit = (function() {
    var noop = Function.prototype;
    return  window.QUnit || (
      window.addEventListener || (window.addEventListener = noop),
      window.setTimeout || (window.setTimeout = noop),
      window.QUnit = load('../vendor/qunit/qunit/qunit.js') || window.QUnit,
      (load('../vendor/qunit-clib/qunit-clib.js') || { 'runInContext': noop }).runInContext(window),
      addEventListener === noop && delete window.addEventListener,
      window.QUnit
    );
  }());

I shim timers for Rhino, setTimeout et all, via qunit-clib.
The addEventListener hack-around is smth I had to add to get it to work in recent versions.
The setTimeout of noop is to fake out QUnit's detections so I can shim it via qunit-clib.
After QUnit is loaded I remove the addEventListener hack-around.

@jzaefferer
Owner

@jdalton thanks for sharing! The addEventListener shim shouldn't be necessary anymore once #401 lands.

@Krinkle
Collaborator
src/
 - intro.js   // header + open closure
 - core.js
 - events.js  // issue #422
 - test.js    // Test constructor
 - assert.js  // QUnit.assert / QUnit.Assertion (issue #374)
 - diff.js    // QUnit.jsDiff
 - dump.js    // QUnit.jsDump
 - reporter/
   - html.js
 - exports.js // window + module.exports
 - outro.js   // end closure
@JamesMGreene
Collaborator

:+1:

What about a Module/Suite constructor?

@jzaefferer
Owner

When we do the split, we should remove the version numbers from source files and add those back as part of the build step. That way releases should become easier as well.

This was referenced
@jzaefferer
Owner

For bower publishing (see #461) we should change the release process to generate the current concatenated files including version numbers, commit that in a branch and tag it. jQuery UI does the same thing.

@Krinkle Krinkle referenced this issue from a commit
@Krinkle Krinkle (WORK IN PROGRESS) The Grand QUnit Split of 2013
Order is mostly preserved, a few changed had to be made (otherwise
we had to create a core-before.js and core-after.js)

- Prototype split
  This used to be in the middle after Test and Assert. It is now
  right after QUnit is first defined.
  So instead of things becoming globals depending on whether
  they're before or after the prototype split, they now become
  globals depending on whether they use QUnit or
  QUnit.constructor.prototype (some parts of the code used this
  already). Removed the, now redundant, comment about the order
  in assert.js (issue #341)

- Test
  Was close to the top inside core, this is now in its own
  modudle after core.
  No side effects (constructor is still hoisted, prototype now
  assigned later, neither is used until the load event).

- Assert
  Was close to the top inside core, this is now in its own
  module after core and Test.
  Removed the `assert` property from the `extend(QUnit, { .. })`
  in core because that now too early (assert is undefined).
  Moved the assignment to assert.js

- Export
  Due to the reliance on position of the prototype split, the
  export for browser globals was somewhere in the middle and
  the one for CommonJS modules on the bottom. They are now both
  at the bottom.

Build process:

- Based on the build process of jQuery and jQuery UI.

Clean up:
- gitignore: Alphabetized.
- gitignore: 'dist/' was already in there apparently, removed
  redundant slash (matching jQuery).
- package.json: Match order of properties with jQuery.
- package.json: Removed 'contributors' property.
- grunt: Removed old build-git task, like jquery, we don't
  include the git hash. Instead we have 'pre' in the version and
  an exact timestamp.
- src: Simplified file headers to be more like jQuery and made
  them match between js and css.

Fixes #378.
3dfe784
@Krinkle Krinkle referenced this issue from a commit
@Krinkle Krinkle The Grand QUnit Split of 2013
Order is mostly preserved, a few changed had to be made (otherwise
we had to create a core-before.js and core-after.js)

- Prototype split
  This used to be in the middle after Test and Assert. It is now
  right after QUnit is first defined.
  So instead of things becoming globals depending on whether
  they're before or after the prototype split, they now become
  globals depending on whether they use QUnit or
  QUnit.constructor.prototype (some parts of the code used this
  already). Removed the, now redundant, comment about the order
  in assert.js (issue #341)

- Test
  Was close to the top inside core, this is now in its own
  modudle after core.
  No side effects (constructor is still hoisted, prototype now
  assigned later, neither is used until the load event).

- Assert
  Was close to the top inside core, this is now in its own
  module after core and Test.
  Removed the `assert` property from the `extend(QUnit, { .. })`
  in core because that now too early (assert is undefined).
  Moved the assignment to assert.js

- Export
  Due to the reliance on position of the prototype split, the
  export for browser globals was somewhere in the middle and
  the one for CommonJS modules on the bottom. They are now both
  at the bottom.

Build process:

- Based on the build process of jQuery and jQuery UI.

Clean up:
- gitignore: Alphabetized.
- gitignore: 'dist/' was already in there apparently, removed
  redundant slash (matching jQuery).
- package.json: Match order of properties with jQuery.
- package.json: Removed 'contributors' property.
- grunt: Removed old build-git task, like jquery, we don't
  include the git hash. Instead we have 'pre' in the version and
  an exact timestamp.
- src: Simplified file headers to be more like jQuery and made
  them match between js and css.

Fixes #378.
8ee826e
@Krinkle Krinkle was assigned
@Krinkle Krinkle referenced this issue from a commit
@Krinkle Krinkle The Grand QUnit Split of 2013
Order is mostly preserved, a few changed had to be made (otherwise
we had to create a core-before.js and core-after.js)

- Prototype split
  This used to be in the middle after Test and Assert. It is now
  right after QUnit is first defined.
  So instead of things becoming globals depending on whether
  they're before or after the prototype split, they now become
  globals depending on whether they use QUnit or
  QUnit.constructor.prototype (some parts of the code used this
  already). Removed the, now redundant, comment about the order
  in assert.js (issue #341)

- Test
  Was close to the top inside core, this is now in its own
  modudle after core.
  No side effects (constructor is still hoisted, prototype now
  assigned later, neither is used until the load event).

- Assert
  Was close to the top inside core, this is now in its own
  module after core and Test.
  Removed the `assert` property from the `extend(QUnit, { .. })`
  in core because that now too early (assert is undefined).
  Moved the assignment to assert.js

- Export
  Due to the reliance on position of the prototype split, the
  export for browser globals was somewhere in the middle and
  the one for CommonJS modules on the bottom. They are now both
  at the bottom.

Build process:

- Based on the build process of jQuery and jQuery UI.

Clean up:
- gitignore: Alphabetized.
- gitignore: 'dist/' was already in there apparently, removed
  redundant slash (matching jQuery).
- package.json: Match order of properties with jQuery.
- package.json: Removed 'contributors' property.
- grunt: Removed old build-git task, like jquery, we don't
  include the git hash. Instead we have 'pre' in the version and
  an exact timestamp.
- src: Simplified file headers to be more like jQuery and made
  them match between js and css.

Fixes #378.
d8c01dc
@Krinkle Krinkle referenced this issue from a commit
@Krinkle Krinkle The Grand QUnit Split of 2013
Order is mostly preserved, a few changed had to be made (otherwise
we had to create a core-before.js and core-after.js)

- Prototype split
  This used to be in the middle after Test and Assert. It is now
  right after QUnit is first defined.
  So instead of things becoming globals depending on whether
  they're before or after the prototype split, they now become
  globals depending on whether they use QUnit or
  QUnit.constructor.prototype (some parts of the code used this
  already). Removed the, now redundant, comment about the order
  in assert.js (issue #341)

- Test
  Was close to the top inside core, this is now in its own
  modudle after core.
  No side effects (constructor is still hoisted, prototype now
  assigned later, neither is used until the load event).

- Assert
  Was close to the top inside core, this is now in its own
  module after core and Test.
  Removed the `assert` property from the `extend(QUnit, { .. })`
  in core because that now too early (assert is undefined).
  Moved the assignment to assert.js

- Export
  Due to the reliance on position of the prototype split, the
  export for browser globals was somewhere in the middle and
  the one for CommonJS modules on the bottom. They are now both
  at the bottom.

Build process:

- Based on the build process of jQuery and jQuery UI.

Clean up:
- gitignore: Alphabetized.
- gitignore: 'dist/' was already in there apparently, removed
  redundant slash (matching jQuery).
- package.json: Match order of properties with jQuery.
- package.json: Removed 'contributors' property.
- grunt: Removed old build-git task, like jquery, we don't
  include the git hash. Instead we have 'pre' in the version and
  an exact timestamp.
- src: Simplified file headers to be more like jQuery and made
  them match between js and css.

Fixes #378.
919fb58
@Krinkle Krinkle closed this issue from a commit
@Krinkle Krinkle The Grand QUnit Split of 2013
Order is mostly preserved, a few changed had to be made (otherwise
we had to create a core-before.js and core-after.js)

- Prototype split
  This used to be in the middle after Test and Assert. It is now
  right after QUnit is first defined.
  So instead of things becoming globals depending on whether
  they're before or after the prototype split, they now become
  globals depending on whether they use QUnit or
  QUnit.constructor.prototype (some parts of the code used this
  already). Removed the, now redundant, comment about the order
  in assert.js (issue #341)

- Test
  Was close to the top inside core, this is now in its own
  modudle after core.
  No side effects (constructor is still hoisted, prototype now
  assigned later, neither is used until the load event).

- Assert
  Was close to the top inside core, this is now in its own
  module after core and Test.
  Removed the `assert` property from the `extend(QUnit, { .. })`
  in core because that now too early (assert is undefined).
  Moved the assignment to assert.js

- Export
  Due to the reliance on position of the prototype split, the
  export for browser globals was somewhere in the middle and
  the one for CommonJS modules on the bottom. They are now both
  at the bottom.

Build process:

- Based on the build process of jQuery and jQuery UI.

Clean up:
- gitignore: Alphabetized.
- gitignore: 'dist/' was already in there apparently, removed
  redundant slash (matching jQuery).
- package.json: Match order of properties with jQuery.
- package.json: Removed 'contributors' property.
- grunt: Removed old build-git task, like jquery, we don't
  include the git hash. Instead we have 'pre' in the version and
  an exact timestamp.
- src: Simplified file headers to be more like jQuery and made
  them match between js and css.

Fixes #378.
6f8f6c3
@Krinkle Krinkle closed this in 6f8f6c3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.