Fix the build issue in core-lro testing #29707
Conversation
MaryGao
changed the title
| @@ -2255,15 +2255,16 @@ matrix( | |||
| }, | |||
| }); | |||
| assert.equal(serialized, expectedSerialized); | |||
| assert.equal(poller.operationState.status, "succeeded"); | |||
| assert.isNotNull(poller.operationState); | |||
| assert.equal(poller.operationState!.status, "succeeded"); | |||
There was a problem hiding this comment.
@jeremymeng Is it possible we could report this error during dev-tool run test:vitest ...?
There was a problem hiding this comment.
It reports them as warnings already just not failing the tests. We could potentially have some additional step to type-check tests and samples-dev code. I will think about this.
@maorleger @mpodwysocki I remember there's some vitest assertion that could allow us to get rid of the !? assert.exists or something similar?
|
API change check API changes are not detected in this pull request. |
| assert.isNotNull(poller.operationState); | ||
| assert.equal(poller.operationState!.status, "succeeded"); |
There was a problem hiding this comment.
I see the issue now. assert.isUndefined() above makes the compiler to infer the type as undefined. And the later assertion won't help any more when operating on undefined type.
I think assert.exists() is still better? as isNotNull asserts that the value is not null.
| assert.isNotNull(poller.operationState); | |
| assert.equal(poller.operationState!.status, "succeeded"); | |
| assert.exists(poller.operationState); | |
| assert.equal(poller.operationState.status, "succeeded"); |
fixes #29634