Add migrated tests and infra to run on uberjenkins#189
Conversation
vipbhardwaj
requested review from
pasin and
torcolvin
and removed request for
torcolvin
There was a problem hiding this comment.
Look good in general. The dev tests also have the test spec for each test. The test spec allows the other to review your test plan (ideally before writing the tests) and to provide a bit info about the purpose of the test. The test spec would also allow the test developers to have a second thought whether the test is really needed or maybe duplicate with the other test. I think it's a good idea for QE to do that as well.
Sample : https://github.com/couchbaselabs/couchbase-lite-tests/tree/main/spec/tests/dev_e2e
|
I have added the specs @pasin , along with a couple of more migrated tests |
|
I have added a few more tests and the specs related to them. |
borrrden
left a comment
borrrden
left a comment
There was a problem hiding this comment.
I will start with the spec and other things before getting to the actual test implementation. Also please rebase to get a clean GH actions run.
vipbhardwaj
force-pushed
the
cbl_migration
branch
from
11d02fe to
4ae7fc6
Compare
borrrden
left a comment
borrrden
left a comment
There was a problem hiding this comment.
Made progress, but needs a whole lot more detail in the spec. Saying things like "start a replicator" is unhelpful. What are the characteristics of the replicator? See the other specs for an example.
borrrden
left a comment
borrrden
left a comment
There was a problem hiding this comment.
I've made it through the first two test suites (delta_sync and no_conflicts)
…gateway.py to be used by test
vipbhardwaj
force-pushed
the
cbl_migration
branch
from
131abd0 to
4391611
Compare
| await cblpytest.sync_gateways[0].get_document( | ||
| "posts", "large_doc", collection="_default.posts" | ||
| ) | ||
| assert "404" in str(excinfo.value) or "returned 404" in str(excinfo.value) |
There was a problem hiding this comment.
This is not the behavior I designed in sync gateway's get_document. Is that what actually happens? It should return None instead of raising an exception.
| @pytest.mark.min_couchbase_servers(1) | ||
| class TestReplicatorEncryptionHook(CBLTestClass): | ||
| @pytest.mark.asyncio(loop_scope="session") | ||
| async def test_replication_complex_doc_encryption( |
There was a problem hiding this comment.
This test is very similar to test_encrypted_push in the dev_e2e suite so I think these two tests should be consolidated and one of them removed.
borrrden
left a comment
borrrden
left a comment
There was a problem hiding this comment.
Assuming that these tests reliably pass I think this is good to go, but I don't like the sgw throwing an exception for a 404. It should just return null and it is probably a bug that it does not. I'll try to address that later.
|
Yes. I have confirmed the fact that all the tests are running thoroughly, and passing. Thank you for an amazing review, I've learnt alot during the review process. |
3 participants
This commit has the first set of migrated tests, as well as the infra needed to run it on uberjenkins, which is in compliance with the directory and file structure being used by dev_e2e.