Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Allow custom comparisons of property values #33

Merged
merged 11 commits into from
May 5, 2017
Merged

Conversation

kitsonk
Copy link
Member

@kitsonk kitsonk commented May 3, 2017

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

This PR adds some features to enable custom comparisons of property values, specifically in harness.expectRender():

  • Add the class support/compare/CustomDiff, allowing the comparison of non-plain objects and custom comparison of primitives.
  • support/compare/diff and support/compare/patch to support the ConstructRecord which allows for diffing and patching of non-plain Objects.
  • Add support/d/compareProperty which wraps a function that returns true or false into a CustomDiff function.

As properties are compared via diff, if it encounters a CustomDiff it will off-load the comparison. In the case of rendering, instead of returning a patch record, the CustomDiff will simply throw when the test function returns false indicating the value is unexpected.

Resolves #32

@codecov
Copy link

codecov bot commented May 3, 2017

Codecov Report

Merging #33 into master will increase coverage by 0.54%.
The diff coverage is 98.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   98.03%   98.58%   +0.54%     
==========================================
  Files          10       10              
  Lines         610      705      +95     
  Branches      137      167      +30     
==========================================
+ Hits          598      695      +97     
+ Misses          6        4       -2     
  Partials        6        6
Impacted Files Coverage Δ
src/support/d.ts 100% <100%> (ø) ⬆️
src/support/compare.ts 99.5% <100%> (+0.1%) ⬆️
src/support/assertRender.ts 97.14% <85.71%> (+4.45%) ⬆️
src/harness.ts 98.32% <0%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4a4d7c...8653784. Read the comment docs.

dylans
dylans previously requested changes May 5, 2017
Copy link
Member

@dylans dylans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very minor grammar nits

*/
export interface UnamedConstructRecord {
/**
* Any arguments to pass the constructor function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any arguments to pass the constructor function -> Any arguments to pass to the constructor function ?

@@ -101,14 +148,32 @@ export interface SpliceRecord {
}

/**
* A record that describes how to instantiate a new object via a constructor function
* @param Ctor The constructor function
* @param args Any arguments to be passed the constructor function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any arguments to be passed the constructor function -> Any arguments to be passed to the constructor function ?

src/support/d.ts Outdated
@@ -20,6 +22,20 @@ export function assignProperties(target: WNode | HNode, properties: WidgetProper
}

/**
* Creates a function which when placed in an expected render will call the `callback`. If the `callback` returns `true`, the value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creates a function which when placed in an expected render will call the callback -> Creates a function which, when placed in an expected render, will call the callback

src/support/d.ts Outdated
@@ -20,6 +22,20 @@ export function assignProperties(target: WNode | HNode, properties: WidgetProper
}

/**
* Creates a function which when placed in an expected render will call the `callback`. If the `callback` returns `true`, the value
* of the property is considered equal, otherwise it is considerd not equal and expected render will fail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and expected render will fail. -> and the expected render will fail.

@dylans dylans added this to the 2017.05 milestone May 5, 2017
prototype: object;
}

export interface ConstructDescriptor {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want some tsdoc for this exported interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

Copy link
Member

@agubler agubler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small doc nit but otherwise looks to add the inline comparison that we need for complex, unknown properties.

@kitsonk kitsonk dismissed dylans’s stale review May 5, 2017 17:21

These have been addressed

@kitsonk kitsonk merged commit 9e6a46f into master May 5, 2017
@kitsonk kitsonk deleted the feature-complex-compare branch May 5, 2017 18:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants