COMPASS 156 Range Component - queryToParams #546

pzrq · 2016-10-28T05:32:28Z

@rueckstiess Please review, thanks.

rueckstiess · 2016-10-30T06:49:12Z

test/validation.store.test.js

  });

-  it('goes into {fetchState: "error"} when receiving an invalid validator doc', function(done) {
+  it.skip('goes into {fetchState: "error"} when receiving an invalid validator doc', function(done) {


what about this and the other skipped tests? are they no longer valid?

Basically these two it.skip worked in the isolated repo so I don't know whether it's something changed as part of the migration into Compass, or something specific to Compass causing them to error: 636a0a5

If you are 100% sure we aren't going back to a shared repo, i.e. these will stay in internal-packages then we should probably delete these 2 skipped tests if you can't see an easy way to fix them (I can't).

Of the other skipped tests in npm run ci:

The context.skip ones work once the range.js rule is included in index.js, so they are skipped in 2f4a406 as a byproduct and will pass when this feature is turned on in COMPASS-235, i.e. a valid use of skip as they are only temporarily failing because of this temporary feature flag.

The simulate click skip never worked, we just didn't know because we got lucky when we tested it once and got the passing race condition instead of the failing race condition because we didn't include a done() in our group programming session.

Expected npm run ci output if you revert 2f4a406 (pending COMPASS-235):

@rueckstiess Actually scratch my earlier advice, it looks like all the goes into {fetchState: ZZZ tests sometimes fail, sometimes pass https://travis-ci.com/10gen/compass/builds/34022022

rueckstiess · 2016-10-30T06:57:37Z

test/validation.store.test.js

+    });
+
+    // Bad as users couldn't insert any documents into the collection
+    it('finds empty range "5 < value <= 5" is not useful', function() {


"finds ... is not useful" - I don't quite understand that sentence. Can we rephrase all of these to make clearer what we test for? Maybe use accepts/rejects as verbs and structure like this:

describe('ValidationStore', () => { describe('Range Rule', () => { context('when passing in values that are valid on the server and rule builder UI', () => { it('rejects empty ranges (5 < value <= 5)', () => {...}); ... }); }); });

Will do, naming things 👍

Revert "Revert "Temporarily add range rule in" so partial progress stays hidden" This reverts commit cb9a1cc.

I don't understand where these are coming from, so skip them for now as it's unclear if we might want to go back to a separate shareable repo in future: FIRST ERROR -------------------- ✓ goes into {fetchState: "error"} when receiving an error back TypeError: Cannot set property 'validator' of undefined at Store._deconstructValidatorDoc (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:81:7) at _fetchFromServer (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:223:27) at Timeout._onTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:14:14) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) TypeError: Cannot set property 'validator' of undefined at Store._deconstructValidatorDoc (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:81:7) at _fetchFromServer (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:223:27) at Timeout._onTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:14:14) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) 1) goes into {fetchState: "error"} when receiving an invalid validator doc Failed with exit code: 1 SECOND ERROR ------------------------- ✓ goes into {fetchState: "error"} when receiving an error back - goes into {fetchState: "error"} when receiving an invalid validator doc TypeError: Cannot set property 'validator' of undefined at Store._deconstructValidatorDoc (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:81:7) at _fetchFromServer (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:223:27) at Timeout._onTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:14:14) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) TypeError: Cannot set property 'validator' of undefined at Store._deconstructValidatorDoc (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:81:7) at _fetchFromServer (/Users/pzrq/Projects/compass/src/internal-packages/validation/lib/stores/index.js:223:27) at Timeout._onTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:14:14) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) 1) goes into {fetchState: "error"} when the result is not an object { AssertionError: expected 1 to equal 2 at Timeout.setTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:110:35) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) message: 'expected 1 to equal 2', showDiff: true, actual: 1, expected: 2 } AssertionError: expected 1 to equal 2 at Timeout.setTimeout (/Users/pzrq/Projects/compass/test/validation.store.test.js:110:35) at tryOnTimeout (timers.js:224:11) at Timer.listOnTimeout (timers.js:198:5) 2) "after each" hook

Lowers the complexity of queryToParams from 14 to 12.

Fixes TODO.

Infinity is dropped as the empty clause is better and so we don't have to represent it in the GUI.

At a mongo shell: use validation; > db.runCommand({collMod: 'validation', validator: {"age": {$gte:0.01,$lt:5000 }, "phone_no": { "$regex": "^[0-9 ]+$", "$options": "mx" } }});

Full range component to come in a JIRA linked to COMPASS-156 or COMPASS-235.

Coming in COMPASS-235, definitely working on that today so we can stop this back and forth partial stuff.

Wasn't found in earlier string replace in COMPASS-156

pzrq added the ready for review label Oct 28, 2016

rueckstiess reviewed Oct 30, 2016

View reviewed changes

pzrq force-pushed the COMPASS-156-query-to-params branch from c048900 to 4ff26fd Compare November 1, 2016 00:14

pzrq and others added 17 commits November 1, 2016 11:15

COMPASS 156: DV - range component queryToParams

7a6bdb6

Revert "Revert "Temporarily add range rule in" so partial progress stays hidden" This reverts commit cb9a1cc.

Migrate validation.rule.test.js into Compass CI tests

85bed5e

Migrate range code and tests from compass-validation repo

6c20225

Refactor logic into validateKeyAndValue

d80d091

Lowers the complexity of queryToParams from 14 to 12.

Remove chained if to lower complexity

f131d48

Fixes TODO.

Drop infinity support and add some negative and decimals tests

6436f84

Infinity is dropped as the empty clause is better and so we don't have to represent it in the GUI.

Improve test context

7d0fc93

Get values from an existing validatorDoc shown in GUI

453a746

At a mongo shell: use validation; > db.runCommand({collMod: 'validation', validator: {"age": {$gte:0.01,$lt:5000 }, "phone_no": { "$regex": "^[0-9 ]+$", "$options": "mx" } }});

Get {<, <=, >, >=, none} server validatorDoc shown in dropdown GUI

93be7cb

At a mongo shell: use validation; > db.runCommand({collMod: 'validation', validator: {"age": {$gte:0.01,$lt:5000 }, "phone_no": { "$regex": "^[0-9 ]+$", "$options": "mx" } }});

Hide range component again and skip integration tests

6ae3422

Full range component to come in a JIRA linked to COMPASS-156 or COMPASS-235.

Structure & naming things request from @rueckstiess, thanks!

ff1b31f

Skip things again

6e15c30

Coming in COMPASS-235, definitely working on that today so we can stop this back and forth partial stuff.

Drop unnecessary skips

10448bb

fix “fetching” test, allow callback to be called once

f1809ba

Fix RangeInput test warnings about missing non-default required "value"

61219eb

Drop "are not useful"

4ff26fd

Wasn't found in earlier string replace in COMPASS-156

rueckstiess merged commit 37be329 into master Nov 1, 2016

rueckstiess deleted the COMPASS-156-query-to-params branch November 1, 2016 01:02

rueckstiess removed the ready for review label Nov 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

COMPASS 156 Range Component - queryToParams #546

COMPASS 156 Range Component - queryToParams #546

Uh oh!

pzrq commented Oct 28, 2016

Uh oh!

rueckstiess Oct 30, 2016

Uh oh!

pzrq Oct 30, 2016

Uh oh!

pzrq Oct 30, 2016 •

edited

Loading

Uh oh!

pzrq Oct 30, 2016

Uh oh!

rueckstiess Oct 30, 2016

Uh oh!

pzrq Oct 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

COMPASS 156 Range Component - queryToParams #546

COMPASS 156 Range Component - queryToParams #546

Uh oh!

Conversation

pzrq commented Oct 28, 2016

Uh oh!

rueckstiess Oct 30, 2016

Choose a reason for hiding this comment

Uh oh!

pzrq Oct 30, 2016

Choose a reason for hiding this comment

Uh oh!

pzrq Oct 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pzrq Oct 30, 2016

Choose a reason for hiding this comment

Uh oh!

rueckstiess Oct 30, 2016

Choose a reason for hiding this comment

Uh oh!

pzrq Oct 30, 2016

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pzrq Oct 30, 2016 •

edited

Loading