Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct validation path, and increase timeout waiting for servers to come online #141

Merged
merged 2 commits into from
Feb 3, 2015

Conversation

dylancwood
Copy link
Contributor

Issues

This PR addresses two existing issues:

#140 Exercises use inconsistent wait times when waiting for servers to come online

Problem

(see issue for more complete context) Sometimes one or both of the servers has not come online when the test attempts to execute.

Solution

Make the timeout for all exercises 2000ms. (commit a6845fa)

#130 the "validation" exercise tests the wrong path

Problem

(see issue for more complete context) The validation exercise asks the user to create and endpoint at /chickens/{breed}, but the exercise tests for an endpoint at /.

Solution

Change the URL used by the validation exercise.js file to include the /chickens path. (commit 11cdb01)

Testing

#140

I was unable to reproduce #140 when using a local installation of makemehapi however, I was able to constently reproduce the issue with a global installation using the following steps:

  1. npm install -g makemehapi
  2. run makemehapi and select the directories excercise
  3. run
    makemehapi verify /usr/local/lib/node_modules/makemehapi/exercises/directories/solution/solution.js

When using the makemehapi master branch, I consistently get something like the following output:

✗ Error connecting to http://localhost:8884: ECONNREFUSED

Error: connect ECONNREFUSED
    at errnoException (net.js:904:11)
    at Object.afterConnect [as oncomplete] (net.js:895:19)

When using my fork/branch, the verification runs successfully.

#130

In order to reproduce this issue, it is necessary to use a hapi server with an endpoint for / as well as for /chickens. Here's a script to use:

/**
 * makemehapi_validation_solution.js
 */
 var Hapi = require('hapi');
var Joi = require('joi');


var server = new Hapi.Server();

server.connection({
    host: 'localhost',
    port: Number(process.argv[2] || 8080)
});

server.route({
    method: 'GET',
    path: '/',
    config: {
        handler: function(request, reply) {
            reply('you asked for this??');
        }
    }
});

server.route({
    method: 'GET',
    path: '/chickens/{breed}',
    config: {
        handler: function (request, reply) {
            reply('You asked for the chicken ' + request.params.breed);
        },
        validate: {
            params: {
                breed: Joi.string().required()
            }
        }
    }
});

server.start();

Inside a local copy of makemehapi/master, save this script as makemehapi_validation_solution.js, then verify it with:

  1. run node makemehapi, and select the validation exercise
  2. run node makemehapi.js verify makemehapi_validation_solution.js

Expected output:

Your submission results compared to the expected:

────────────────────────────────────────────────────────────────────────────────

1.    ACTUAL:  "you asked for this??"
1.  EXPECTED:  "{\"statusCode\":404,\"error\":\"Not Found\"}"

2.    ACTUAL:  ""
2.  EXPECTED:  ""


────────────────────────────────────────────────────────────────────────────────

✗ Submission results match expected

# FAIL

Your solution to VALIDATION didn't pass. Try again!

Next, checkout this fork/branch and rerun the same commands. It should now pass.


function error (err) {

exercise.emit('fail', 'Error connecting to http://localhost:' + port + ': ' + err.code)
exercise.emit('fail', 'Error connecting to ' + url + ': ' + err.code)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch. :)

@nvcexploder nvcexploder added this to the 1.0.3 milestone Feb 3, 2015
nvcexploder added a commit that referenced this pull request Feb 3, 2015
Correct validation path, and increase timeout waiting for servers to come online
@nvcexploder nvcexploder merged commit d5d2ed7 into ccarruitero:master Feb 3, 2015
@dylancwood
Copy link
Contributor Author

Thanks for the merge!
When you have a chance, please push 1.0.3 to npm :-)

@nvcexploder
Copy link
Contributor

Yes! I will push it out either later this evening or tomorrow, definitely.

On Mon, Feb 9, 2015 at 3:46 PM, Dylan Wood notifications@github.com wrote:

Thanks for the merge!
When you have a chance, please push 1.0.3 to npm :-)


Reply to this email directly or view it on GitHub
#141 (comment).

@nvcexploder nvcexploder modified the milestones: 1.1.0, 1.0.3 Mar 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants