Strider-CD / strider-simple-runner

Easy-to-configure in-process Runner implementation for Strider.
MIT License
3 stars 18 forks source link

Huge rewrite #14

Closed jaredly closed 11 years ago

jaredly commented 11 years ago

I essentially rearranged a ton of things, and then rewrote some things too.

Major cool things:

selection_022

All of the testing I did went without a hitch, but there's definitely more testing to be done.

Fixes Strider-CD/strider#146 Fixes Strider-CD/strider#135

niallo commented 11 years ago

Reviewing & testing this, looks great so far, get back to you soon

niallo commented 11 years ago

npm test fails under OS X for me. It is related to pty.js and environment variables. Tests pass under linux though.

Seeing if I can fix it.

jaredly commented 11 years ago

hmm interesting. Tests run fine for me on OSX 10.7.5

On Thu, Jul 25, 2013 at 4:23 PM, niallo notifications@github.com wrote:

npm test fails under OS X for me. It is related to pty.js and environment variables. Tests pass under linux though.

Seeing if I can fix it.

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/pull/14#issuecomment-21589194 .

niallo commented 11 years ago

Ok with my latest changes, tests pass on OS X and Linux. This way of doing things should be a little more POSIX-compatible too for random OSes (*BSD, Solaris/SmartOS, etc)

jaredly commented 11 years ago

Do you really need to stringify the ENV? child_process.spawn claims to take care of that for you. What was the issue?

On Thu, Jul 25, 2013 at 4:48 PM, niallo notifications@github.com wrote:

Ok with my latest changes, tests pass on OS X and Linux. This way of doing things should be a little more POSIX-compatible too for random OSes (*BSD, Solaris/SmartOS, etc)

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/pull/14#issuecomment-21590388 .

niallo commented 11 years ago

Yes, basically pty.js's "env" param was not being honoured properly on OS X 10.8 - that or /bin/{ba}sh env pass thru is non-standard - and test 6 was failing.

niallo commented 11 years ago

Can you actually start up Strider (HEAD from github) with this?

I see the following:

$ npm start

> strider@1.3.1 start /Users/niallo/projects/strider
> bin/strider

25 Jul 16:55:12 - info: Using MongoDB URL: mongodb://localhost/strider-foss
25 Jul 16:55:12 - info: Looking for webapp-type extensions under /Users/niallo/projects/strider/node_modules
25 Jul 16:55:12 - info: strider-custom webapp extension loaded
25 Jul 16:55:12 - info: strider-env webapp extension loaded
25 Jul 16:55:12 - info: strider-sauce webapp extension loaded
25 Jul 16:55:12 - info: Error loading extension: TypeError: Cannot read property 'log' of undefined in directory: /Users/niallo/projects/strider/node_modules/strider-simple-worker
npm ERR! weird error 1
npm ERR! not ok code 0
jaredly commented 11 years ago

strange; yeah I'm having no problem. OSX, HEAD of strider and strider-simple-worker. Have you npm linked?

On Thu, Jul 25, 2013 at 5:56 PM, niallo notifications@github.com wrote:

Can you actually start up Strider (HEAD from github) with this?

I see the following:

$ npm start

strider@1.3.1 start /Users/niallo/projects/strider bin/strider

25 Jul 16:55:12 - info: Using MongoDB URL: mongodb://localhost/strider-foss 25 Jul 16:55:12 - info: Looking for webapp-type extensions under /Users/niallo/projects/strider/node_modules 25 Jul 16:55:12 - info: strider-custom webapp extension loaded 25 Jul 16:55:12 - info: strider-env webapp extension loaded 25 Jul 16:55:12 - info: strider-sauce webapp extension loaded 25 Jul 16:55:12 - info: Error loading extension: TypeError: Cannot read property 'log' of undefined in directory: /Users/niallo/projects/strider/node_modules/strider-simple-worker npm ERR! weird error 1 npm ERR! not ok code 0

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/pull/14#issuecomment-21593170 .

niallo commented 11 years ago

Yes. Is there something you haven't committed?

jaredly commented 11 years ago

Ah I see. Just pushed up the one I missed to strider HEAD. passing in a "logger" to plugins. Ought to work now.

On Thu, Jul 25, 2013 at 6:08 PM, niallo notifications@github.com wrote:

Yes. Is there something you haven't committed?

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/pull/14#issuecomment-21593663 .

niallo commented 11 years ago

Sweet - it starts now :)

jaredly commented 11 years ago

Another thing that we get: live updates of heroku pushes.

niallo commented 11 years ago

I'm not sure why yet, but this breaks usage with strider-sauce + strider-jelly. Works fine when I use current version strider-simple-worker. I'm going to see if I can track it down.

28 Jul 21:25:19 - info: Domain caught error while processing undefined is not a function TypeError: undefined is not a function
    at Function.<anonymous> (/Users/niallo/projects/strider-simple-worker/node_modules/gitane/index.js:87:7)
    at next (/Users/niallo/projects/strider-simple-worker/node_modules/step/lib/step.js:51:23)
    at ChildProcess.exithandler (child_process.js:641:7)
    at ChildProcess.EventEmitter.emit (events.js:98:17)
    at maybeClose (child_process.js:735:16)
    at Socket.<anonymous> (child_process.js:948:11)
    at Socket.EventEmitter.emit (events.js:95:17)
    at Pipe.close (net.js:466:12)
28 Jul 21:25:19 - error: Unexpected server error: { [TypeError: undefined is not a function]
  domain:
   { domain: null,
     _events: { error: [Function] },
     _maxListeners: 10,
     members: [] },
  domainThrown: true } undefined is not a function TypeError: undefined is not a function
    at Function.<anonymous> (/Users/niallo/projects/strider-simple-worker/node_modules/gitane/index.js:87:7)
    at next (/Users/niallo/projects/strider-simple-worker/node_modules/step/lib/step.js:51:23)
    at ChildProcess.exithandler (child_process.js:641:7)
    at ChildProcess.EventEmitter.emit (events.js:98:17)
    at maybeClose (child_process.js:735:16)
    at Socket.<anonymous> (child_process.js:948:11)
    at Socket.EventEmitter.emit (events.js:95:17)
    at Pipe.close (net.js:466:12) undefined Error
    at EventEmitter.<anonymous> (/Users/niallo/projects/strider-simple-worker/lib/job.js:80:101)
    at EventEmitter.emit (events.js:95:17)
    at Domain.<anonymous> (/Users/niallo/projects/strider-simple-worker/lib/worker.js:69:10)
    at Domain.EventEmitter.emit (events.js:95:17)
    at process._fatalException (node.js:249:27)
28 Jul 21:25:19 - info: done 500
niallo commented 11 years ago

Ok I have tracked this through a bunch. The rewrite assumes that if no detection rules match, then there are no plugins. It is fine for detection rules to not match - registered build hooks should still run though. I will comment on the exact lines.

niallo commented 11 years ago

With the commits I added (detailed explanations above), strider-sauce and strider-jelly work with the rewrite!

There is still one thing I'd like to track through, probably related to PTY, which is this error that I don't normally see with strider-sauce:

screen shot 2013-07-29 at 12 31 09 am

jaredly commented 11 years ago

Awesome, thanks for really looking into this.

niallo notifications@github.com wrote:

With the commits I added (detailed explanations above), strider-sauce and strider-jelly work with the rewrite!

There is still one thing I'd like to track through, probably related to PTY, which is this error that I don't normally see with strider-sauce:

screen shot 2013-07-29 at 12 31 09
am


Reply to this email directly or view it on GitHub: https://github.com/Strider-CD/strider-simple-worker/pull/14#issuecomment-21703512

Sent from my Android device with K-9 Mail. Please excuse my brevity.