ZIP Task Addition

[ctalkington]

I have created a task for zipping files that works pretty well at the moment. think the only thing it could really use is better error pickup and some unit tests but its a start. i also have a RECESS task that I will be submitting soon.

[tkellen]

This looks like a good fit for contrib, thanks for the submission! Could you please make a test case? Testing with grunt is a little cumbersome at the moment, but you shouldn't have much trouble figuring out how we're doing it if you check out the current test/ dir.

Also, just FYI, I am out of the country at the moment and I won't be around much for the next week starting tomorrow.

[ctalkington]

@tkellen i will see about putting a test together this weekend.

[tkellen]

Hey Chris--

I'm back in the US and I just wanted to drop a note to say that I'd still love to see this included in contrib if a test is included. Thanks!

[tkellen]

Hey Chris--

Just checking in again to see if you'll be completing the test for this task so we can include it?

[ctalkington]

Sorry been between paid work and moving but I intend to get to it in the next week.

[tkellen]

No worries--I just wanted to make sure this didn't get lost in the shuffle. I try not to leave PRs dangling for too long.

[ctalkington]

@tkellen glad for that, its my fault this time. do you think compressing a zip and comparing md5s would suffice, or do you have another test in mind.

[tkellen]

Confirming with a MD5 hash sounds good to me. If you find the MD5 hash changing with every run, the mtime of the zipped files is probably being included. Assuming that is the case, you can just compress/inflate something and confirm that it worked.

[ctalkington]

@tkellen didnt even think about that mtime! but guess comparing a source directory and extracted to directory is a good test. might need to see about making an unzip in that case. will see how i go.

[bkardell]

I've been using this, I'd like to see it included. If tests are preventing, please let me know and I will contribute some.

[bkardell]

Yeah, kind of irks me that I couldn't do it through github itself...
The interface is all there, It should have sent you a proposed change
with commit notes, but it just winds up at 404.

On Fri, Jun 1, 2012 at 2:52 PM, Chris Talkington
reply@reply.github.com
wrote:

that was fun, first time i've rebased and squashed a pull request.

i'm looking into tests atm, wanted to get current code first.

---

Reply to this email directly or view it on GitHub:
#13 (comment)

[ctalkington]

@tkellen having some issues with tests on windows. what is the common approach to using tests? I am able to just do npm install and grunt test on most anything I have built so far but this comes up with a module not defined MS JScript warning.

[tkellen]

I don't use windows much these days, but I do have a VM handy that I can try your test on. Can you make a new branch and link me to it so I can give it a run?

[ctalkington]

@tkellen the issue that i can't test my tests at the moment to even build them out, as grunt command doesn't work at either level (ie root dir and tests dir) also tried node grunt.js test just gives blank output.

[tkellen]

You should be able to run npm install followed by npm test from the root of this repo. What is happening when you do?

[tkellen]

Hmm, this looks like an issue for grunt itself.

[tkellen]

Actually scratch that, maybe not. Hmm, I don't have an answer for you right off and I don't have time to look into this further today. Sorry :/

[tkellen]

Great! If you include the zip test in this PR, I will merge it. I will have to look into the windows issues later--they definitely are limited to your stuff.

[ctalkington]

I have to say this test setup is a major pain. So many callbacks and
trying to get my file stat to work but its one hell of a chore.

[tkellen]

Whoops, I meant to say aren't limited to your stuff. Testing with grunt is a bit of a pain right now, I know. Also, I might be able to offer some pointers on your test once I see it.

[bkardell]

Excellent! Now that's what I call service :)

On Fri, Jun 1, 2012 at 5:19 PM, Tyler Kellen
reply@reply.github.com
wrote:

Whoops, I meant to say aren't limited to your stuff.  Testing with grunt is a bit of a pain right now, I know.  Also, I might be able to offer some pointers on your test once I see it.

---

Reply to this email directly or view it on GitHub:
#13 (comment)

[ctalkington]

proposed fixes for windows moved to #24. cleaning this thread of related comments.

[ctalkington]

ok. so once I have finished with #25 and its been merged, I will need to do one last rebase and squash on this before its merge-able into master. we might have to come up with another way to test too, line endings are the nightmare now but I think by making it all one line as long as its not edited, its count should be fine. might look into using something binary that is less likely to vary. only time will tell.

[ctalkington]

think this is good to be merged. done about all the testing/optimization i can take for the moment.

[tkellen]

Looks good! One issue: the task doesn't handle the destination directory not existing.

The test fails, also. Seems to be the same issue as the options test. I'll look more tonight!

[ctalkington]

@tkellen will look into the directory issue. i don't have node setup on a my linux server otherwise I'd be able to better test that end of things.

[ctalkington]

@tkellen think we can merge this now. let it roam in the wild for a bit so we can get any feedback etc.

[tkellen]

done and done. i'll cut a release in the morning. off to bed here. have a good one :)