nathanboktae / mocha-phantomjs

:coffee: :ghost: Run client-side mocha tests in the command line through phantomjs
MIT License
954 stars 112 forks source link

Add --ignore-resource-errors switch to hide resource errors from output #169

Closed andreialecu closed 9 years ago

andreialecu commented 9 years ago

This fixes #153 by adding an --ignore-resource-errors switch to hide resource errors.

nathanboktae commented 9 years ago

I would not use -i but only --ignore-resource-errors . its verbose but this option should be rare - 404s are generally bugs unless its testing handing of these conditions.

Also a test for this please.

andreialecu commented 9 years ago

In the tests we're running we call an api via ajax, and the api replies with non 200 codes. It may reply with 404 when an item is not found, which is something we test for.

We also test for malformed input which also makes the api reply with non 200 codes.

These api calls show up in logs.

See here for an example: https://travis-ci.org/deployd/deployd/builds/45888892 Scroll to the bottom.

I will rename it, but I'm not sure how to test it or if a test is really relevant since the change is so minor and straight forward.

andreialecu commented 9 years ago

I pushed another commit where it is renamed as per your suggestion @nathanboktae

Regarding tests, I looked a bit at them and I don't know how to test this without a web server. All the tests seem to be local right now, and resource errors only happen with remote requests. Any ideas?

nathanboktae commented 9 years ago

Yeah I was thinking about the test, and it is tricky to implement as we do not have a dependency outside the box for the CI build, nor a server running during tests. Also nor do we have tests for the existing functionality! I'll take a look at it later.

Also I did double-check and mocha uses -i to invert grep results, so we'll implement that when -g gets implemented (tracked in #145)

nathanboktae commented 9 years ago

Thought I already left this comment but I didn't realize resource errors trigger for XHRs - I only thought they triggered for missing resources like scripts, images, CSS. Now it makes it a more common scenario. Maybe the shorthand could be -I - thoughts @metaskills ?

metaskills commented 9 years ago

TL;DR - Just pick anything that makes sense.

Names of args is hard :( which side do we lean too. I think there are seldom good answers and we should just be careful and cognitive. So first side (mocha) or (phantomjs) to implement the need is maybe our winner :)

andreialecu commented 9 years ago

By the way, a new release having this new functionality would be great so we can plug it into deployd and have our Travis logs look cleaner. :)

nathanboktae commented 9 years ago

You can point directly at master for the moment. I started working on the test for this and it's easy with a bad css, script, or image url, so I'll add it soon.

meeh0w commented 9 years ago

Is there a way to use this option within grunt config object with grunt-mocha-phantomjs plugin? E.g.:

mocha_phantomjs: {
  all: {
    options: {
      '--ignore-resource-errors': 'true'
    }
  }
}

Sorry if a silly question, I'm new to all this.

nathanboktae commented 9 years ago

Not true - we are using it in our Gruntfile at work. remove the --:

    mocha_phantomjs: {
      all: ['tests.html'],
      options: {
        'ignore-resource-errors': true
      }
    }
meeh0w commented 9 years ago

@nathanboktae yes, after a lot of tries (camelCase, single dash at the beginning) I went through the code and figured it out. I forgot to update my post for others, though. Thanks!