Added a config for backward compat #13

Merged
merged 3 commits into from Apr 13, 2012

Conversation

Projects
None yet
3 participants
Owner

davglass commented Feb 22, 2012

This is for @nzakas.

I added a config to YUITest.TestRunner to allow it to not throw on tests with no Assertions.

There are several test suites inside YUI that do not use Asserts since they use the "_should" assertions to tell if something throws or errors.

This just removes that extra assert when the config option is set.

nzakas was assigned Feb 22, 2012

nzakas commented on 24fecdd Feb 22, 2012

The run() method accepts an options object that defines how the tests should be run. I think this would be better as an option on that object.

Owner

davglass replied Feb 22, 2012

I didn't add it there because I wanted all tests to abide by this out of the box. With our large system, I didn't want to have to modify every single test module to add this parameter for backward compatibility. This gives a "one stop" place to set this and have it "just work" as it did before that assert was added.

I understand the intent, the problem is that this would create two places to specify options instead of one. I don't like the idea of having a global configuration for the TestRunner because your tests may have different results depending on the TestRunner being used...that should never happen. Also, having tests without asserts is a horrible practice, and I don't want to encourage that.

That being said, I'm willing to compromise with you. If you want this property, mark it as private and deprecated, and prefix it with an underscore. That way, at least it will be hidden or flagged in docs as something people shouldn't use. It can be a YUI-only thing.

Owner

davglass replied Feb 22, 2012

That works! On it..

Owner

davglass commented Feb 22, 2012

Pushed up the change, and as an FYI, I added a ton of tests to the YUI source tree for YUITest:

https://github.com/yui/yui3/tree/master/src/test/tests

We might be able to make them reusable for both projects.

@davglass davglass Added version check to "flush" work around
    In 0.6.x the drain event does not fire, which means
    the process.exit(code) is never executed.
    This prohibits automated scripts from failing on a failed
    test.
1d06aa5
Owner

davglass commented Apr 12, 2012

@nzakas Can you merge these in? The flush fix I just added makes using YUITest in a CI environment impossible with the latest stable Node.

Contributor

nzakas commented Apr 13, 2012

I don't have access to the YUI account for merging in YUI Test
changes. Ryan always handled the merges for me.

Owner

davglass commented Apr 13, 2012

Can you merge it into yours and publish a new npm package with the fix?

Once it's merged with yours, then i can merge it and push it to the yui account.

Contributor

nzakas commented Apr 13, 2012

Sure, can you send the pull request to my fork?

Not sure how quickly I can get to it (still traveling), but I'll do my best.

Owner

davglass commented Apr 13, 2012

Understood, take your time. Get to it when you can man..

Contributor

nzakas commented Apr 13, 2012

Found a little time (just for you). Pushed 0.7.3.

yuibuild merged commit 1d06aa5 into yui:master Apr 13, 2012

Owner

davglass commented Apr 13, 2012

You Rock!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment