Skip to content

Migrate remaining chaos/MTLS/CRUD tests for edge server#397

Open
barkha06 wants to merge 5 commits into
mainfrom
edge_server_tests
Open

Migrate remaining chaos/MTLS/CRUD tests for edge server#397
barkha06 wants to merge 5 commits into
mainfrom
edge_server_tests

Conversation

@barkha06
Copy link
Copy Markdown
Contributor

  • Introduced CA-based TLS and mTLS support
  • Added certificate generation utilities and updated ES setup to provision CA, server, and client certs
  • Added mTLS test coverage
  • Added chaos tests: test_edge_server_offline_sync_and_recovery and test_edge_server_with_concurrent_rest_requests
  • Added Bulk doc CRUD test

@barkha06 barkha06 requested a review from borrrden April 23, 2026 15:11
@barkha06 barkha06 self-assigned this Apr 23, 2026
8. Restart Edge Server 3 and wait for it to come online.
9. Create 10,000 documents in `travel.hotels` via bulk create on Edge Server 3.
10. Verify documents are created on Edge Server 3 and wait for replication to become idle.
11. Kill Edge Server 3 again.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the purpose of step 11?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It validates that the newly created documents have been fully propagated and stored across the replication.Also by killing ES3 again after replication reaches idle, we ensure no buffered data is lost. It's an attempt to test replication after source removal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like the result would be the same without it, would it not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, do you think it would make more sense if i wait for ES2 (which replicates with ES3) to be idle and kill ES3 and then wait the other edge-servers to be idle

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not hurting anything I guess, but I don't see the point. I am always wary of creating tests with too many steps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay then I'll remove the kill edge-server step

Copy link
Copy Markdown
Member

@borrrden borrrden left a comment

Choose a reason for hiding this comment

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

Overall the test steps should be a little more specific about what they are doing. When I write the markdown, I try to have the steps in the markdown be identical to the test steps in the test. This helps a lot with not having rot and accidentally drifting away from the spec intention.

Comment thread tests/QE/edge_server/test_chaos_scenarios.py Outdated
Comment thread tests/QE/edge_server/test_chaos_scenarios.py Outdated
@barkha06 barkha06 requested a review from borrrden April 24, 2026 06:27
@barkha06 barkha06 changed the title Migrate remain chaos/MTLS/CRUD tests for edge server Migrate remaining chaos/MTLS/CRUD tests for edge server Apr 24, 2026
self.mark_test_step("get server information")
self.mark_test_step("get server information with TLS")
version = await edge_server.get_version()
self.mark_test_step(f"VERSION:{version}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like it should be a normal log message, not a test step.

edge_server = await cblpytest.edge_servers[0].configure_dataset(
db_name="names", config_file=f"{SCRIPT_DIR}/config/test_mtls_config.json"
)
self.mark_test_step("get server information with TLS")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With mTLS right?

return True

except Exception as e:
return {"error": str(e), "optype": optype, "doc_id": doc_id}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The rest of this function returns boolean but this returns a dict. That's strange, and how does it work with assert?

revmap=revmap,
)

self.mark_test_step("Final consistency validation")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should say what it is validating

self.mark_test_step("Run randomized CRUD workload")
for i in range(1000):
op = random.choice(operations)
self.mark_test_step(f"Iteration {i}: {op}")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Logging message, not test step

]
await edge_server.bulk_doc_op(bulk_ops, db_name="db")

self.mark_test_step("Verify initial document load")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verify how (i.e. write that it is checking the count against the expected count)

async def test_multiple_doc_crud(
self, cblpytest: CBLPyTest, dataset_path: Path
) -> None:
self.mark_test_step("test_multiple_doc_crud")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a test step. As a rule of thumb, each test step should have one or more corresponding asserts before the next one.

)
updated.append(all_docs.rows[i + 50].id)

self.mark_test_step("Execute bulk document operations")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could combine all of the following test steps written here into one: Execute bulk document operations, verify creations updates and deletions.

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.

2 participants