Skip to content

Add links to reference material #19

rmurphey opened this Issue May 1, 2012 · 8 comments

6 participants

rmurphey commented May 1, 2012

The tests should include links to reference material so people can get some direction if they are stuck.


I have a question related to this—certain tests (such as in arrays.js) are very specific, to the point of providing arguments. I feel that these are more "friendly" than, say, the functions tests, certainly because as you mention there is no reference material but also because it is unclear whether you are supposed to have the liberty of adding your own parameters to the functions. "makeClosures" in particular seems vague to me, there's a lot of reference you could link to on how to use closures that might not actually help somebody pass that test.

In your opinion, is this a separate issue (functions should have arguments defined to serve as structure/hints), or was the intent to leave them as-is and provide more specific references to compensate?


This is a good point; the stub functions in app/functions.js and the tests in tests/app/functions.js should probably match as far as the arguments. Let me know if you'd like to submit a pull request for this :)

@kadamwhite kadamwhite added a commit to kadamwhite/js-assessment that referenced this issue Jun 17, 2012
@kadamwhite kadamwhite Match stub function definitions in /app with args from /tests/app
Based on the discussion of #19, it makes the tests more "user friendly" to
have the arguments in the stub functions match those in the tests (except
in cases such as curryIt or useArguments, where the test depends on
proper use of unspecific arguments)

I can add some notes -- I still have to google most of this stuff anyway ;) If I add comments to my answers branch can you find a way to incorporate them?


we've agreed to add MDN links. PR forthcoming.

jmeas commented May 14, 2015

A first step to get things started here could be to audit the source and create a list of all of the places that could maybe use references.

@therobinkim therobinkim added a commit that referenced this issue Dec 20, 2015
@rmurphey initial files
wip on tests

moar tests

adding readme

fix heading level

readme tweaks

readme tweaks

readme tweaks

changing language describing tests

fix package.json details

flow control, module tests wip; more objects tests

better test for object property iteration

fix #1 -- pass arg to be removed to fn

fix #2 -- speak function must return something

ensure bin/serve is executable (fix #3)

Switched order of greeting and name in object a so that the 'alter the context in which a method runs' test makes more sense.

Update name and repo in package.json

Also, you should totally `npm publish` this so that people can
install it and run it.

updating expected results of 'change the context in which a function is called' as speak appends three exclamation marks.

better description

marking v0.1.0

fleshing out package.json

adding instructions

Added test for function arguments and currying

Added closure example

Values are now randomized + minor fixes

Added test for applying functions

Fix expectation to match reality, now and in the future :)

fix #12, no need to install npm separately

Fix: ensure that functions really are applied

Update `bin/serve` with a she-bang line so it can be executed in the command line (as per the docs).

Made the concatination test more specific

I found the wording to be a bit too loose. I also added an element from the
initial array which prevents the problem to being solved by computing the
union (which could be addded as a separate test case).

More array tests

remove unused var

link todos to issues page instead

add explainer, use _.each instead

command line runner working

conditionally add browser tests

tests running in browser and on command line

update readme

Added flowControl and modules to runner.js

Added the expectations for the "you should be able to conditionally branch your code" test under flowControl.

removed the comment about adding your own tests

add `npm test` script

separate arrays answers

eliminate underscore dependency

separate async answers

separating flow control answers

add amdefine

separate functions answers

separate modules answers

separating objects answers

removing views tests

they're dumb and backbone-specific

update instructions

Bugfix: Correct call to answer method

add html5 doctype

add test to find all occurrences of an item in an array

fix #25 update README with correct instructions

fix #26 typo in test

Re-expose doSomeStuff to the makeClosures function that was moved to a new scope in b421915

Update object.js - insert missing variable

Adding .DS_Store to gitignore

Signed-off-by: Kelly Miyashiro <>

Pass sayIt to argsAsArray, add fn param to argsAsArray. Otherwise it is impossible.

Signed-off-by: Kelly Miyashiro <>

Fix functionFunction test

It was perfectly possible to pass the test by doing the following code:

functionFunction : function() {
    return function () { return 'Hello, world'}

While observing the expectation declared, I don't think that's the
purpose of this test, so by just adding a second expectation, it
becomes possible to avoid cheats like the one stated previously.

Match stub function definitions in /app with args from /tests/app

Based on the discussion of #19, it makes the tests more "user friendly" to
have the arguments in the stub functions match those in the tests (except
in cases such as curryIt or useArguments, where the test depends on
proper use of unspecific arguments)

List jQuery as included.

Fix small typo

move tests to separate it blocks for clarity

Remove duplicate assertion in curry test

Fix typo in logical operators test

Unify answer templates

  * Include `define` definition everywhere
  * Do not require underscore

Clarify arrays.remove test case

this test passes incorrectly

    makeClosures : function(array,doSomeStuff) {

Update to mocha 1.2.x to allow clickable tests in browser reporter.

better fizzbuzz tests to avoid obvious but wrong solutions

Clarify modules test

Make sure that the passed functions are called

Without looking at the test’s source, I for one ignored the first argument
to functions.partial.

Move sayItCalled = false to a beforeEach

make clear that you can use jquery for async answers

fix fizzbuzz in line 21 or 25

remove comma, add instructions for missing num case

better promises test

remove outdated comments

move closures test up above currying, partials

reorder answers to match tests file

mark async tests done correctly

add link to answers repo

initial work on some best practices tests

adding some basic regex tests

fix bestPractices expect dependency problem,fix window var when running npm test

another indexOf test to make the goal more clear

require expect in the test, not the answers

fix async.js test to require amdefine and expect, too

move window declaration to test

Add assertions to existing tests

These assertions enforce the expected behaviour a bit more strictly

Add test for monetary formatting (USD)

fix #48

arrays removeWithoutCopy

minor cleanup of new array test

Fixed test description to reflect test

description text was reversed, showing the OR text for the AND tests, and vice versa.

switch to using chai instead of expect

Adds flow control NaN test to expect false

recursion tests

numbers-related tests

adding permutation test

better parseInt test

upgrade to require 2.1.2

add function test case "you should be able to create a 'partial' function for variable number of applied arguments"

change 'curry function' test case to return single-argument function when there are still argument(s) to be provided

linting with grunt

better fizzbuzz instructions

use .jshintrc file instead for editor support

enforce single quotes

fix #60

Dont expect order for test on Finding All Occurrences in array

I ran into a problem with this test as I was using a decremental for loop to iterate through the array and pick out results. Adding sort() to the assertion allows for the results to be returned in any order.

adds tests for shift and unshift

adds stubs for prepend and shorten

changes 'shorten' to 'curtail'

Add a value to the arrays duplicates test array.

The test array had only pairs of duplicates, now it has a triplet of 3s.

whitespace fixes

counter test

add count.cancel test

better function name

Updated styling of stats bar to handle scrolling on smaller window sizes.

Updated z-index of stats to help it have precedence

Upgrade to mocha 1.12.0

count.js grunt lint fixes:
* expr:true
* global console:true

ReadMe Driven Development

npm install deps

livereload snippet in runner

gruntfile watch & livereload config

count faster

* 600 milliseconds

add irc channel info

Added tests to logical operators

Exisitng tests pass when bitwise operators are used

fixed order of logicalOperators tests to match the layout of the app, similar to the rest of the tests

fixed whitespace

Catch array count solution using [].reduce with missing initialvalue

Added Gitter badge

update jquery 1.7 ==> 1.11.2

.then > .pipe > .then that doesn't .pipe

[close #91]

Clarified ambiguous test wording for arrays.js

remove and removeWithoutCopy functions were unclear on whether
to remove a single instance or all instances of a given value

Before, wording was unclear in arrays.js tests for remove and
removeWithoutCopy that the intent is not to remove a specific value, or
a single instance of an value, but rather all instances of a value from
an array.

Update ambiguous text for expected behavior in test wording for

[close #83]

[close #80]

Update readme to list Chai instead of expectjs

[close #85]

Replace console.log after modifying it for count test

[close #87]

additional tests for recursion

[close #86]

Fix jshint errors in count test

use a var statement for every variable declaration

Refactor to use serve-static, not express

bump to version 0.2.0

Prevent library from being published to npm.

add serve-static and finalhandler to package, remove mocha dupe

update node download link in README

Update copyright date, license

Update async test description.

Updated default IP address for local server

This is due to a "bug" in Chrome that looks like it won't be fixed.

Resolves rmurphey/js-assessment#62

Squashed commits in order to resolve rmurphey/js-assessment#115

Resolves rmurphey/js-assessment#62

Per @PAkerstrand's provided solution, this commit disallows a user
from returning the result of a logical operator rather than the
operation expression itself.

Resolves rmurphey/js-assessment#62

This commit includes the assertion checking for use of bitwise operators
which was previously left out of the PR commit.

re-add assertion checking for bitwise operator

Manage browser deps in package, not local libs

- remove local dep files
- add pacakges to package.json
- update runner.js and runner.html to use node_modules/
- update app/config to use node_modules/

convert answers to object literals, attach to window

conditionally export to global or window depending on env

Remove require from tests

- unwrap tests from define and closure
- if in node, require answer file and chai expect
- change 'answers' var to category specifc global, e.g. arraysAnswers
- remove require from runner.html, add answers and tests files

remove require config files

remove require js dep, update npm test script

Fix some errors found in rmurphey/js-assessment#112

- `npm install load-grunt-tasks --save`
- add globals to .jshintrc
- minor refactor of `exports` assignment in answers
- a couple other small syntax issues (semicolon, et al)

Add jQuery dependency to package.json

Remove RequireJS references from README.

Further updates to README regarding dependencies.

Fix failing test case under logical operators.

bump to version 0.3.0

improve array duplicates test

Remove duplicate assertion

In the remaining assertion you can clearly see that there are
three elements in the array.

In my mind, removing this assertion removes a bit of duplication
in the tests. This makes them a bit better than they were before.

Futher simplify remaining assertion

Removing the join() preserves the visibility of the return type
of the function.

[close #132]

Add sinon.js to make use of fake timer

With the intent to resolve #114 per @jmeas' suggestion, this
commit adds sinon.js and modifies the count.js test to use fake timers.

re-add the "run tests" comment in runner.html

Tick the clock every 1/10th of a second

We change our expectations here, in that we now explicitly count
every 1/10th of a second by making use of sinon's fake timer.

explicitly call tick(100) rather than store as t

remove done call, as it is unnecessary now

remove answers for count -- accidentally committed

Add some string related tests

As just a random example of why this is still an issue - I'm currently looking at curtail in arrays.js. I did a quick Google thinking that perhaps it was something I had just not heard of, and I've still got absolutely no idea what is meant here. I think along with reference links, maybe even just a one line comment per test could be useful. Most folks know indexOf, but imagine this:

//Given an array and a value, return the position in the array where the value is found, or -1 if not found at all.

Closing this due to age and ambiguity. Pull requests welcome to point people to good docs!

@rmurphey rmurphey closed this Apr 26, 2016
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.