add in replicated execution #2513
Conversation
There was a problem hiding this comment.
Why are we stopping and deleting the canister here?
There was a problem hiding this comment.
I don't think we should be implementing anything on the ic object anymore, let's remove this and all experimental ic object code
src/lib/experimental/ic/index.ts
Outdated
There was a problem hiding this comment.
Let's not implement this in the experimental ic object
| } | ||
| } | ||
|
|
||
| @inspectMessage |
There was a problem hiding this comment.
Make sure falling through won't give us false positives
There was a problem hiding this comment.
inspectMessage does not run in replicated mode
There was a problem hiding this comment.
7 ways to Sunday this test should not have false positives
There was a problem hiding this comment.
Do you not think we should have this run multiple times at least like the other property tests?
There was a problem hiding this comment.
As in, instead of the for loops, we actually use fast check...deploying might harm this idea?
There was a problem hiding this comment.
If we really wanted that we could make sure that it would still work with deploying.
However, the point of using fast check to do num runs is 1. to make sure we have different values each run, and 2 to be able to make sure that those values are different than the previous runs. Since we don't need that and we don't need to change how many runs we do arbitrarily, I feel like we can just pick a number we fill good with and just do that every time. If I'm being honest I feel like the for loop is a little excessive. I don't want to use fast check just to make it feel more like a prop test. It is a prop test. The property we are testing is that every time inReplicatedExecution is called in X environment it returns Y. Where X is a very specific enumerable environment and Y is a very specific deterministic one to one result. It is a prop test, it just doesn't need arbitrariness to prove it, says I.
| for (let i = 0; i < 10; i++) { | ||
| expect( | ||
| await actor.getInspectMessageIsInReplicatedExecution() | ||
| ).toBe(true); |
| ).toStrictEqual([true]); | ||
| }); | ||
|
|
||
| it('redeploys the canister', async () => { |
|
|
||
| expect( | ||
| await actor.getHeartbeatIsInReplicatedExecution() | ||
| ).toStrictEqual([true]); |
There was a problem hiding this comment.
Why does this use toStrictEqual([true]) when other areas in this file do not? I think we should unify
Finishing touches
Contributor
Reviewer