Added missing test suite

[aaronsofaly]

You can see the test suite working here, http://aaronsofaly.github.io/chai-docs/api/test/.

There's a bug related to the syntax highlighting on that page though.

The /api/test.md file includes this line: Use it to confirm that Chai will function correctly in your browser environment (should does not work in IE9).

On page load, the should syntax highlighting for that line functions correctly. But as soon as the tests are done running, the character encoding around the should changes and then you see <span class="nx">should</span>.

You can confirm this by visiting the test page, see all the tests run, then scroll all the way back to the top to see the character encoding issue.

The old site doesn't have this issue as the syntax highlighting for that site doesn't add css classes and spans.

I've tried to debug what is causing the issue but so far I haven't been able to figure out what's causing it. I'll continue to debug it and send another PR if I can fix it.

[lucasfcosta]

Hi @aaronsofaly, thanks for the awesome PR! This is looking really great 😄

I'll take a look at this right now and leave some comments if needed. Thanks again for this one!

[lucasfcosta]

Well, I just went through it all and I would really like to thank you for this! Good job, we've been needing this for quite some time 😄

This looks really good, however I see that your tests (assert for example) aren't matching the ones for v3.5.0. I think they're missing the last commit, since the difference is that your test files don't have tests for includeDeepMembers. It would be good to update those files with all the commits on 3.5.0.

The tests seem to be failing because there's no way Travis can push to this repo. As a side note I would also like to point that maybe a new build routine would be interesting in order for the automatic checks on GitHub to work properly.

[aaronsofaly]

Thanks @lucasfcosta!

Interesting bit about assign vs capture.

I wanted to focus this PR on porting the Jade templates from the old_site repo to the new Jekyll site. So I haven't made any optimizations. I got the tests from the old_site repo found here. They're out of date so I'll look into updating them to the newest versions.

_layouts/test.html from this PR comes from porting the Jade templates found here and here.

I agree with you on updating the build routine.

[aaronsofaly]

Thanks @lucasfcosta for pointing me to version 3.5.0 of the tests. This PR now includes a commit which updates the tests to 3.5.0.

[lucasfcosta]

Great job @aaronsofaly! I just pointed out that thing about assign and capture so we won't forget for further versions (it was just a side note), let's keep this one inside the scope 😄

Thanks for updating the tests, I'm very happy to have this PR. This one LGTM, let's wait for @meeber or @keithamus' approval to merge this 👍

[keithamus]

Great work on the PR @aaronsofaly.

I'm tempted to sign this off, but I wonder if it is worth us making the effort to automate using the tests from the latest chai release, rather than having to manually copy+commit them for each release? We could either use rawgithub.com or perhaps copy the files as part of the makefile, similar to the other generated data? Thoughts on this?

[aaronsofaly]

Hey @keithamus!

Yes, I'm actually trying to figure out the best way to automate this as having to copy and paste the tests on each Chai release is kind of a pain.

I was originally thinking we could copy the tests from Chai's npm module using the Makefile. But then I realized Chai's npm module excludes the tests.

So then I thought maybe using Chai as a git submodule. The old_site repo had this but the new site doesn't. So I decided against this because I figured there was a reason it wasn't done this way on the new site.

So I'm trying to explore what else we could do. I haven't heard of rawgithub.com before, but it looks interesting. I'll look into maybe going that route.

[keithamus]

Hey @aaronsofaly that's awesome, glad to see you're thinking about the problem to.

The way we could use rawgithub.com, is simply to point to the test JS files from a master branch, like so:

https://rawgit.com/chaijs/chai/master/test/assert.js https://rawgit.com/chaijs/chai/master/test/expect.js https://rawgit.com/chaijs/chai/master/test/should.js

I'm not sure if we would perhaps run into any CORS conflicts or something? A potential pitfall of doing this would be that the master tests may deviate in a way that makes them fail between the latest release and master which could be a problem.

[lucasfcosta]

@keithamus unfortunately I think that this won't be enough.

As you said if we do this we would run the tests on the master branch against the last released version. This would make them incompatible. IMO we're going to use the latest tests on masterwe should use the Chai version on the master branch too, otherwise the tests on the docs page won't reflect the state of the actual version at all, which is the main goal of that page.

Regarding the CORS conflict, I found this on StackOverflow explaining how to work around that and it seems that, as you've said, using rawgit.com is enough.

IMO the best way to do this would be to include the test files from the latest tag tree when building the Docs. Here is a link explaining how to do that. What do you think about it?

[keithamus]

@lucasfcosta the good news is, we already have the latest chai version available for us in the generated _data/releases.json!

If we want to point to a specific tag, rawgit.com allows us to do this easily; for example https://rawgit.com/chaijs/chai/3.5.0/test/assert.js

So in Jekyll, we should be able to do something a bit like...

<script src="https://cdn.rawgit.com/chaijs/chai/{{ site.data.releases[0].tag_name }}/test/assert.js"></script>

That should be enough to solve all of the problems mentioned; we don't need any additions to the makefile, we don't need to manually update tests, and this should work even with CORS.

[lucasfcosta]

@keithamus That's great! I think we won't find a better way than this one to do this 😄 Good idea

[aaronsofaly]

That's an awesome solution @keithamus. I'll update this PR.

Also, the version of mocha.js that the test page uses is 3 years old. When I test the site using the newest mocha.js version, the <span class="nx">should</span> issue described above goes away.

[aaronsofaly]

I've added 2 new commits to the PR. You can see it live here, http://aaronsofaly.github.io/chai-docs/api/test/.

The first commit uses cdn.rawgit.com to host the tests and the chai.js file. I then removed the tests from the repo.

The second commit upgrades mocha to the latest version. This also fixes the <span class="nx">should</span> described in the OP.

I have a few questions though.

There's a file called globalShould.js in Chai's 3.5.0 tree. Should that be added to the tests page as well?

Now that the chai.js file is hosted elsewhere, is the chai.js file located at the root of the repo an orphaned file now? The make file builds it. You can see it on this line here. The .gitignore file is supposed to be ignoring it though. You can see that line here.

[keithamus]

Amazing work @aaronsofaly! Few small comments above. Also...

There's a file called globalShould.js in Chai's 3.5.0 tree.

I think we should add this. If you would be so kind as to add it to your PR that'd be awesome. Address this and the comments above, and I'll be happy to merge this.

[aaronsofaly]

Sounds good @keithamus! I'll update this PR shortly.

[aaronsofaly]

I added a new commit which moves the chai.js file to the head template and added globalShould.js to the tests page. http://aaronsofaly.github.io/chai-docs/api/test/.

I didn't remove chai.js from the Makefile because I just noticed that the _includes/banner.html file links to download Chai at http://chaijs.com/chai.js.

[aaronsofaly]

I just noticed a separate issue. The chaijs.com site doesn't have the left sidebar for the guide, http://chaijs.com/guide/. I haven't looked into it yet but the code is here.

[lucasfcosta]

@aaronsofaly good job! This already looks good to me (but I'm not going to use the magic word for this yet) because I'd like to confirm with @keithamus if we're going to tackle the new issue you've just found on this PR. IMO we keep this PR into the missing tests scope and then open another one and make changes to that sidebar. However, I'm not sure it would look good on that page, but that's something we can discuss later.

For now I think this is going really well :smile:

[keithamus]

I agree. Let's raise a new PR for the new issue. For now this PR does what it is meant to, and so it LGTM!

[lucasfcosta]

LGTM too!

[aaronsofaly]

I agree, I'll create a new PR for the guide issue!

[keithamus]

Need to fix the LGTM setup it seems 😕