workshopper / levelmeup

Level Me Up Scotty! An intro to Node.js databases via a set of self-guided workshops.
Other
271 stars 60 forks source link

New workshopper (attempt #2) #79

Closed martinheidegger closed 7 years ago

martinheidegger commented 7 years ago

Okay this is a crazy rewrite of most of levelmeup. It is ...

It would be lovely if someone could take the time to review it.

jpwesselink commented 7 years ago

I've started reviewing this, I'll let you know tomorrow!

ralphtheninja commented 7 years ago

A lot of commits. I think I'll just do the workshop and not focus as much on reviewing the code.

ralphtheninja commented 7 years ago

@martinheidegger I noticed two things while running the tests.

  59 passing (56s)
  2 failing

  1) Testing exercises ./test/basics_put/valid_01.js (3:1)       :
     Uncaught 
  rror
      at /home/lms/src/workshopper/levelmeup/node_modules/workshopper-adventure-test/spec/exercises.js:67:15
      at ChildProcess.exithandler (child_process.js:213:5)
      at emitTwo (events.js:106:13)
      at ChildProcess.emit (events.js:191:7)
      at maybeClose (internal/child_process.js:877:16)
      at Socket.<anonymous> (internal/child_process.js:334:11)
      at emitOne (events.js:96:13)
      at Socket.emit (events.js:188:7)
      at Pipe._handle.close [as _onclose] (net.js:498:12)

  2) Testing exercises ./test/horse_js_tweets/invalid_01.js (7:0)        :
     Uncaught Error:   You have 4 challenges left.  

  Type 'levelmeup' to show the menu.  

      at /home/lms/src/workshopper/levelmeup/node_modules/workshopper-adventure-test/spec/exercises.js:63:15
      at ChildProcess.exithandler (child_process.js:197:7)
      at emitTwo (events.js:106:13)
      at ChildProcess.emit (events.js:191:7)
      at maybeClose (internal/child_process.js:877:16)
      at Process.ChildProcess._handle.onexit (internal/child_process.js:226:5)
jpwesselink commented 7 years ago

Culprit:

Object.keys(obj).forEach(function (key) {
  db.put(key, obj[key])
})
db.close(callback)

Probably has to do with https://github.com/Level/leveldown/issues/32#issuecomment-254081211

ralphtheninja commented 7 years ago

It seems I no longer get the previous output when something is successful:

$ levelmeup verify 01.js             
  You have 11 challenges left.       

  Type 'levelmeup' to show the menu. 

Maybe this is supposed to happen, I'm not sure.

martinheidegger commented 7 years ago

@jpwesselink

@ralphtheninja You mean that there should be more text, right?

martinheidegger commented 7 years ago

I just published https://www.npmjs.com/package/exec-module which should make the execution not just more stable but also allow us to give better error messages. This PR should probably be updated to use exec-module.

ralphtheninja commented 7 years ago

@ralphtheninja You mean that there should be more text, right?

@martinheidegger Yep!

martinheidegger commented 7 years ago

We have Nodeschool Tokyo in a little more than 24h and it would be awesome if it could be reviewed before that

martinheidegger commented 7 years ago

@ixkaito reviewed it and I merged a PR, merging this workshopper with master since there has been no further review.