Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to generate test operations for original values in the first object (continuation) #228

Merged
merged 16 commits into from
Jul 25, 2019

Conversation

warpech
Copy link
Collaborator

@warpech warpech commented Jul 2, 2019

Continuation of #226

Changes that I made compared to the original PR:

  • was: extra boolean argument on "observe" and extra object argument on "generate" and - "compare"
  • is: extra boolean argument "generate" and "compare"
  • changed the word inversible to invertible. Please correct me if I'm wrong
  • added way more tests for test operation generation

Pending questions:

  1. https://github.com/immerjs/immer also registers inverse patches, but they do it in a separate patch, not within the same patch. Is the solution in this PR more or less useful?

Benchmark results:

Before this PR (bbd3002, npm run build && npm run bench-duplex)

add operation
 Ops/sec: 3021278 ±0.33% Ran 154893 times in 0.051 seconds.
remove operation
 Ops/sec: 2467208 ±0.45% Ran 127226 times in 0.052 seconds.
replace operation
 Ops/sec: 2854277 ±0.28% Ran 146362 times in 0.051 seconds.
move operation
 Ops/sec: 271583 ±0.37% Ran 15241 times in 0.056 seconds.
copy operation
 Ops/sec: 214417 ±0.45% Ran 18773 times in 0.088 seconds.
test operation
 Ops/sec: 1316006 ±0.31% Ran 67352 times in 0.051 seconds.
===================
generate operation
 Ops/sec: 1680155 ±0.32% Ran 87099 times in 0.052 seconds.
generate operation and re-apply
 Ops/sec: 1483583 ±0.20% Ran 76006 times in 0.051 seconds.
compare operation
 Ops/sec: 427630 ±0.36% Ran 22101 times in 0.052 seconds.
compare operation same but deep objects
 Ops/sec: 122387928 ±0.31% Ran 6230468 times in 0.051 seconds.

After this PR

add operation
 Ops/sec: 3033581 ±0.26% Ran 155154 times in 0.051 seconds.
remove operation
 Ops/sec: 2513818 ±0.33% Ran 129460 times in 0.051 seconds.
replace operation
 Ops/sec: 2804656 ±0.31% Ran 144192 times in 0.051 seconds.
move operation
 Ops/sec: 271968 ±0.38% Ran 15132 times in 0.056 seconds.
copy operation
 Ops/sec: 215846 ±0.35% Ran 19086 times in 0.088 seconds.
test operation
 Ops/sec: 1311638 ±0.33% Ran 67526 times in 0.051 seconds.
==================
generate operation
 Ops/sec: 1705131 ±0.32% Ran 87963 times in 0.052 seconds.
generate operation and re-apply
 Ops/sec: 1452317 ±1.99% Ran 75998 times in 0.052 seconds.
compare operation
 Ops/sec: 422706 ±0.52% Ran 21843 times in 0.052 seconds.
compare operation same but deep objects
 Ops/sec: 115692461 ±0.71% Ran 6021898 times in 0.052 seconds.
================== new benchmarks start here: ==================
generate operation, invertible = true
 Ops/sec: 1607667 ±0.27% Ran 82412 times in 0.051 seconds.
generate operation and re-apply, invertible = true
 Ops/sec: 1401189 ±0.42% Ran 73320 times in 0.052 seconds.
compare operation, invertible = true
 Ops/sec: 237997 ±0.27% Ran 12237 times in 0.051 seconds.
compare operation same but deep objects, invertible = true
 Ops/sec: 116096872 ±0.50% Ran 5978518 times in 0.051 seconds.

@warpech
Copy link
Collaborator Author

warpech commented Jul 2, 2019

cc @pytlesk4

src/duplex.ts Outdated Show resolved Hide resolved
test/spec/duplexSpec.js Outdated Show resolved Hide resolved
@warpech
Copy link
Collaborator Author

warpech commented Jul 9, 2019

@tomalec please re-review.

I have achieved performing the same test using invertible set as true and then false by creating a new helper functions: variantIt, insertIf and usage of spread operator. I think this is quite neat, similar to NUnit fixtures. I wonder if this is readable, though. Please let me know.

@tomalec
Copy link
Collaborator

tomalec commented Jul 10, 2019

I wonder if this is readable, though. Please let me know.

At least to me it is :)

Copy link
Collaborator

@tomalec tomalec left a comment

Choose a reason for hiding this comment

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

LGTM

@pytlesk4
Copy link
Contributor

@warpech thanks for taking this over the finish line!

@tomalec
Copy link
Collaborator

tomalec commented Jul 11, 2019

@pytlesk4 once again thanks for starting a great PR and sorry for being picky :)

@warpech warpech merged commit 8e59102 into master Jul 25, 2019
@warpech warpech deleted the generate-test-ops-warpech branch July 25, 2019 10:40
@warpech
Copy link
Collaborator Author

warpech commented Jul 25, 2019

Released today as 2.2.0

@pytlesk4
Copy link
Contributor

@warpech can this get published to NPM?

@warpech
Copy link
Collaborator Author

warpech commented Aug 1, 2019

Done, thanks for pinging me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants