FirstLegoLeague / fllscoring

FLL Scoring app. See Demo:
http://firstlegoleague.github.io/fllscoring/
GNU General Public License v2.0
14 stars 19 forks source link

Filesystem hooks mechanism #289

Closed poelstra closed 6 years ago

poelstra commented 6 years ago

Part of #245.

This adds a hooking mechanism to allow transforming data (e.g. scores.json) between receiving it from the browser and actually saving it to disk.

You'll need to run npm install due to new dependencies. Then, run npm test to run the tests, which now also include tests for the newly added server code.

Note that actually using the new hooks mechanism is the subject of future PRs. As such, it should have no noticeable impact on the existing functionality.

kmeesters commented 6 years ago

290 Implemented

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 87.5% when pulling cc73f5e07228036877dcc5bbc65192b9274e88f6 on fs_hooks into 303fb8f7deb5c52397ae43ce394adba792cdee85 on master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 87.5% when pulling bd14ebd3f65fd13f7c22157e8b0e59c332a9967c on fs_hooks into 303fb8f7deb5c52397ae43ce394adba792cdee85 on master.

poelstra commented 6 years ago

Rebased onto master (leaving out the commits that are now in #290), so should be ready.

kmeesters commented 6 years ago

There is currently an error when running NPM test. Failures: 1) file_system callHooks matches against relative paths Message: Expected 'something' to be 'SOMETHING'. Stack: Error: Expected 'something' to be 'SOMETHING'. at UserContext.it (D:\git\fllscoring\server_test\file_systemSpec.js:99:2 8) at <anonymous> at process._tickCallback (internal/process/next_tick.js:188:7)

Changing line 99 in file_systemSpec.js from "SOMETHING" to "something" (lower case) fixes this though. Just want to let you know in case there is more behind this.

poelstra commented 6 years ago

Thanks for testing!

Sure, changing that line will do the trick, but it means that the transform didn't run, so in that case it's indeed supposed to fail :)

Given that the test is about relative paths, I suppose the issue has to do with the fact that Travis and I use Linux, and you're using Windows. I suppose the path is given with backslashes on your platform, whereas I'm always matching against forward slashes.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 87.5% when pulling e8e0375a86dce550218128a0795e2393f3219115 on fs_hooks into 303fb8f7deb5c52397ae43ce394adba792cdee85 on master.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 87.5% when pulling 3770ebd1e952dfb3b36c37136886d8d7f4933e7d on fs_hooks into 303fb8f7deb5c52397ae43ce394adba792cdee85 on master.

poelstra commented 6 years ago

@kmeesters made a change that should hopefully fix it on Windows (untested yet, as I didn't add AppVeyor to this branch yet). Also added some extra tests (no other changes).

kmeesters commented 6 years ago

Tested (Win 10, node 8.5.0)