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

the "validation using joi" assignment does not test for the first criterium #132

Open
Pomax opened this issue Jan 13, 2015 · 4 comments
Open

Comments

@Pomax
Copy link

Pomax commented Jan 13, 2015

The assignment states that if isGuest is false, a username is required, but it clearly does not test for this.

The solution shows the code you wanted to see was along the lines of

validate: {
    payload: Joi.object({
        isGuest: Joi.boolean().required(),
        username: Joi.when('isGuest', { is: false, then: Joi.required() }),
        password: Joi.string().alphanum(),
        accessToken: Joi.string().alphanum()
    }).options({ allowUnknown: true }).without('password', 'accessToken')
}

however, the following code passes just fine (which is just as well, the assignment taught me nothing about how to use Joi, I kind of tried stuff until this assignment passed so I could move on)

validate: {
  payload: Joi.object({
    isGuest: Joi.boolean(),
    username: Joi.string(),
    accessToken: Joi.string().alphanum(),
    password: Joi.string().alphanum()
  })
  .options({allowUnknown: true})
  .without('password', 'accessToken')
}

Even with this, the "official solution" looks like it doesn't even say what type username has to be. I don't dig Joi after these two assignments at all, it's been more revealed as quite a hassle and worth passing up in favour of a more user-friendly POST data validator

@nvcexploder nvcexploder modified the milestone: 1.0.3 Jan 28, 2015
@fiveisprime fiveisprime removed this from the 1.0.3 milestone Aug 14, 2015
@matjack1
Copy link

matjack1 commented Jan 5, 2016

I think exactly the same. But I've had a go trying to add another verification and sounds not so easy with workshopper. Does somebody have any hint on how to do that? I've searched in other workshopper-based projects but I couldn't find anything. Any idea?

@louy2
Copy link

louy2 commented Sep 14, 2016

Source code for reference:
https://github.com/hapijs/makemehapi/blob/master/exercises/validation_using_joi_object/exercise.js

I have a hard time parsing that hack honestly. Seems to me workshopper as a tester UI also managing test constructing and processor is a bit overreach.

@martinheidegger
Copy link

martinheidegger commented Sep 15, 2016

@matjack1 @louy2 first off: I hope to replace workshopper sooner than later with workshopper-adventure. However: technically all three (including adventure) work on the assumption that it is one test. i.e. if the test fails, the lesson is not marked completed. makemehapi uses comparestdout to create the one test. I.e. if different things are written to stdout by the input script vs. the sample script then comparestdout will mark everything as failed. comparestdout is used by learnyounode because all the lessons are based on stdout but other workshoppers/adventures use tap for example to compare tests. That all being said: I think an easy solution would be to just pipe the hyperquest input a combined-stream to test multiple inputs. I, personally, do not like this method of verification for web services and think that makemehapi should use another form of verification than stdout but for a short-term solution it would be good to just use combined stream.

@martinheidegger
Copy link

@matjack1 Sidenote: You can ask me questions about workshopper-adventure anytime. I do not have the capacity to monitor all the requests that come in because they are usually bugs of the workshoppers, but if a question comes in from a developer: you can ask me anytime. I am also available on gitter for private questions https://gitter.im/martinheidegger and we now?! have a workshopper gitter chat: https://gitter.im/nodeschool/workshoppers

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

No branches or pull requests

6 participants