Skip to content

Commit 73b3cec

Browse files
kvinwangkvinwang
committed
Fix unit tests
1 parent fea17be commit 73b3cec

7 files changed

Lines changed: 29 additions & 46 deletions

File tree

‎certbot/src/acme_client.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl AcmeClient {
6464
dns_txt_ttl: u32,
6565
) -> Result<Self> {
6666
let credentials: Credentials = serde_json::from_str(encoded_credentials)?;
67-
let http_client = Box::new(ReqwestHttpClient::new());
67+
let http_client = Box::new(ReqwestHttpClient::new()?);
6868
let account =
6969
Account::from_credentials_and_http(credentials.credentials, http_client).await?;
7070
let credentials: Credentials = serde_json::from_str(encoded_credentials)?;
@@ -84,7 +84,7 @@ impl AcmeClient {
8484
max_dns_wait: Duration,
8585
dns_txt_ttl: u32,
8686
) -> Result<Self> {
87-
let http_client = Box::new(ReqwestHttpClient::new());
87+
let http_client = Box::new(ReqwestHttpClient::new()?);
8888
let (account, credentials) = Account::create_with_http(
8989
&NewAccount {
9090
contact: &[],

‎certbot/src/http_client.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
//! Custom HTTP client for instant_acme that supports both HTTP and HTTPS.
66
7+
use anyhow::{Context, Result};
78
use bytes::Bytes;
89
use http::Request;
910
use http_body_util::{BodyExt, Full};
@@ -22,18 +23,12 @@ pub struct ReqwestHttpClient {
2223

2324
impl ReqwestHttpClient {
2425
/// Create a new HTTP client.
25-
pub fn new() -> Self {
26+
pub fn new() -> Result<Self> {
2627
let client = Client::builder()
2728
.user_agent("dstack-certbot/0.1")
2829
.build()
29-
.expect("failed to build reqwest client");
30-
Self { client }
31-
}
32-
}
33-
34-
impl Default for ReqwestHttpClient {
35-
fn default() -> Self {
36-
Self::new()
30+
.context("failed to build reqwest client")?;
31+
Ok(Self { client })
3732
}
3833
}
3934

‎gateway/src/cert_store.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,13 @@ fn format_expiry(not_after: u64) -> String {
319319
mod tests {
320320
use super::*;
321321

322+
impl CertStore {
323+
/// Check if a certificate can be resolved for a given SNI hostname
324+
pub fn has_cert_for_sni(&self, sni: &str) -> bool {
325+
self.resolve_cert(sni).is_some()
326+
}
327+
}
328+
322329
fn make_test_cert_data() -> CertData {
323330
// Generate a self-signed test certificate using rcgen
324331
use ra_tls::rcgen::{self, CertificateParams, KeyPair};
@@ -427,9 +434,8 @@ mod tests {
427434
assert!(store.has_cert_for_sni("foo.example.com"));
428435
assert!(store.has_cert_for_sni("bar.example.com"));
429436

430-
// Note: wildcard certs also match nested subdomains in our implementation
431-
// This is intentional for ease of use with wildcard domains
432-
assert!(store.has_cert_for_sni("sub.foo.example.com"));
437+
// Wildcard certs do not match nested subdomains
438+
assert!(!store.has_cert_for_sni("sub.foo.example.com"));
433439

434440
// Should not resolve different domain
435441
assert!(!store.has_cert_for_sni("example.org"));

‎gateway/src/main_service.rs‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// SPDX-License-Identifier: Apache-2.0
44

55
use std::{
6-
collections::{BTreeMap, BTreeSet},
6+
collections::{BTreeMap, BTreeSet, HashSet},
77
net::Ipv4Addr,
88
ops::Deref,
99
sync::{Arc, Mutex, MutexGuard},
@@ -528,41 +528,36 @@ fn start_zt_domain_watch_task(proxy: Proxy) {
528528
let kv_store = proxy.kv_store.clone();
529529
let certbot = proxy.certbot.clone();
530530

531-
// Track known domains to detect additions
532-
let known_domains: std::sync::Arc<std::sync::Mutex<std::collections::HashSet<String>>> =
533-
std::sync::Arc::new(std::sync::Mutex::new(
534-
kv_store
535-
.list_zt_domain_configs()
536-
.into_iter()
537-
.map(|c| c.domain)
538-
.collect(),
539-
));
540-
541531
let mut rx = kv_store.watch_zt_domain_configs();
542532
tokio::spawn(async move {
533+
// Track known domains to detect additions
534+
let mut known_domains = kv_store
535+
.list_zt_domain_configs()
536+
.into_iter()
537+
.map(|c| c.domain)
538+
.collect::<HashSet<_>>();
539+
543540
loop {
544541
if rx.changed().await.is_err() {
545542
break;
546543
}
547544

548545
// Get current domains
549-
let current_domains: std::collections::HashSet<String> = kv_store
546+
let current_domains: HashSet<String> = kv_store
550547
.list_zt_domain_configs()
551548
.into_iter()
552549
.map(|c| c.domain)
553550
.collect();
554551

555552
// Find newly added domains
556-
let mut known = known_domains.lock().unwrap();
557553
let new_domains: Vec<String> = current_domains
558554
.iter()
559-
.filter(|d| !known.contains(*d))
555+
.filter(|d| !known_domains.contains(*d))
560556
.cloned()
561557
.collect();
562558

563559
// Update known domains
564-
*known = current_domains;
565-
drop(known);
560+
known_domains = current_domains;
566561

567562
// Trigger renewal for new domains
568563
for domain in new_domains {

‎gateway/src/main_service/tests.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ impl std::ops::Deref for TestState {
2121
async fn create_test_state() -> TestState {
2222
let figment = load_config_figment(None);
2323
let mut config = figment.focus("core").extract::<Config>().unwrap();
24-
let cargo_dir = env!("CARGO_MANIFEST_DIR");
25-
config.proxy.cert_chain = format!("{cargo_dir}/assets/cert.pem");
26-
config.proxy.cert_key = format!("{cargo_dir}/assets/cert.key");
2724
let temp_dir = TempDir::new().expect("failed to create temp dir");
2825
config.sync.data_dir = temp_dir.path().to_string_lossy().to_string();
2926
let options = ProxyOptions {

‎gateway/src/proxy.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::{
1111
};
1212

1313
use anyhow::{bail, Context, Result};
14+
use or_panic::ResultOrPanic;
1415
use sni::extract_sni;
1516
pub(crate) use tls_terminate::create_acceptor_with_cert_resolver;
1617
use tokio::{
@@ -212,14 +213,14 @@ pub fn start(config: ProxyConfig, app_state: Proxy) -> Result<()> {
212213
let rt = tokio::runtime::Builder::new_current_thread()
213214
.enable_all()
214215
.build()
215-
.expect("Failed to build Tokio runtime");
216+
.or_panic("Failed to build Tokio runtime");
216217

217218
let worker_rt = tokio::runtime::Builder::new_multi_thread()
218219
.thread_name("proxy-worker")
219220
.enable_all()
220221
.worker_threads(config.workers)
221222
.build()
222-
.expect("Failed to build Tokio runtime");
223+
.or_panic("Failed to build Tokio runtime");
223224

224225
// Run the proxy_main function in this runtime
225226
if let Err(err) = rt.block_on(proxy_main(&worker_rt, &config, app_state)) {
@@ -239,8 +240,6 @@ mod tests {
239240

240241
#[test]
241242
fn test_parse_destination() {
242-
let base_domain = ".example.com";
243-
244243
// Test basic app_id only
245244
let result = parse_dst_info("myapp").unwrap();
246245
assert_eq!(result.app_id, "myapp");

‎sdk/rust/tests/test_tappd_client.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,18 @@ use std::env;
1111
async fn test_tappd_client_creation() {
1212
// Test client creation with default endpoint
1313
let _client = TappdClient::new(None);
14-
15-
// This should succeed without panicking
16-
assert!(true);
1714
}
1815

1916
#[tokio::test]
2017
async fn test_tappd_client_with_custom_endpoint() {
2118
// Test client creation with custom endpoint
2219
let _client = TappdClient::new(Some("/custom/path/tappd.sock"));
23-
24-
// This should succeed without panicking
25-
assert!(true);
2620
}
2721

2822
#[tokio::test]
2923
async fn test_tappd_client_with_http_endpoint() {
3024
// Test client creation with HTTP endpoint
3125
let _client = TappdClient::new(Some("http://localhost:8080"));
32-
33-
// This should succeed without panicking
34-
assert!(true);
3526
}
3627

3728
// Integration tests that require a running tappd service

0 commit comments

Comments
 (0)