Live test framework

[danieljurek]

No description provided.

[Jinming-Hu]

Hi @danieljurek , some storage unittest cases require additional configuration for the storage account. Some case has a bug (FileShareServiceClientTest.SetProperties). Can we skip these test cases with some command line arguments to ctest to unblock our release?

I disabled test cases for Blob Tags. Please rebase on top of latest master branch.

[danieljurek]

Skipping tests should be done in the test code so it's clear every time tests run that a test is being skipped. Most test frameworks have a concept of "skipped" as a result for a single test. If we start excluding things in the CI then it makes CI more complex and fragile and makes it difficult for outside contributors to know which tests are not expected to pass.

[Jinming-Hu]

Skipping tests should be done in the test code so it's clear every time tests run that a test is being skipped. Most test frameworks have a concept of "skipped" as a result for a single test. If we start excluding things in the CI then it makes CI more complex and fragile and makes it difficult for outside contributors to know which tests are not expected to pass.

I meant disabling some test cases as a workaround, a temporary solution, given we cannot get all test cases passed for now and the release date is so tight.

[weshaggard]

Looks reasonable.

[weshaggard]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

Pending on #872 to be fully functional.

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[vhvb1989]

The test cases are reusing the same http connections.
We should run each test independently (that's how I updated azure core)
Each test should be expected to be an independent case of creating a connection, creating a client and running the test.

Having one single client to run all tests makes tests interfere between them and there could be always some intermittent issues. Like, if a test pass but let the storage client in some specific state that the next test is not expecting.
And when using curl, ech curl connection will be in a pool to be re-used. The server is more likely to close a connection when the channel have been open for a long time (then the retry policy will help us, but what if it does not).

I guess, if you really expect to create this relation between tests cases is fine, but I think that's not a good unit testing model.

When adding ctests, you can just use gtest_add_tests instead of add_gtest and gtest would create one unique test for each of your tests. (see azure core: https://github.com/Azure/azure-sdk-for-cpp/blob/master/sdk/core/azure-core/test/ut/CMakeLists.txt#L45)

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

The test cases are reusing the same http connections.
We should run each test independently (that's how I updated azure core)
Each test should be expected to be an independent case of creating a connection, creating a client and running the test.

Having one single client to run all tests makes tests interfere between them and there could be always some intermittent issues. Like, if a test pass but let the storage client in some specific state that the next test is not expecting.
And when using curl, ech curl connection will be in a pool to be re-used. The server is more likely to close a connection when the channel have been open for a long time (then the retry policy will help us, but what if it does not).

I guess, if you really expect to create this relation between tests cases is fine, but I think that's not a good unit testing model.

When adding ctests, you can just use gtest_add_tests instead of add_gtest and gtest would create one unique test for each of your tests. (see azure core: https://github.com/Azure/azure-sdk-for-cpp/blob/master/sdk/core/azure-core/test/ut/CMakeLists.txt#L45)

I created #890 to track this. We can discuss it there.

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[katmsft]

/azp run cpp - storage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[weshaggard]

/azp run cpp - core

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[weshaggard]

@katmsft looks like the storage tests are now passing. Is this ready to get merged?

[katmsft]

@katmsft looks like the storage tests are now passing. Is this ready to get merged?

Yes, sorry, emailed you about the PR being ready but not caught this comment. Thanks for checking.