autodev: Test Quality Fix: [no_assertion] xplat/proxygen/lib/dns/test/CAresResolverTest.cpp

Summary:
**File:** `xplat/proxygen/lib/dns/test/CAresResolverTest.cpp`

## Summary
Added placeholders for assertions in three tests that currently lack any assertions and only verify that code runs without crashing. These fixes highlight the need to verify observable outcomes of timeoutExpired() and fail() calls to improve test quality. The TestParseTxtRecords test is well-constructed and requires no changes.

## Issues Found & Fixed

## `ScheduleFallbackOnTimeout` (line 69)
**Issue Type:** no_assertion
**Problem:** The test calls timeoutExpired() on a query but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that timeoutExpired() triggers the expected fallback scheduling behavior, such as invoking a callback method or changing query state.
**Fix:** The fix adds a placeholder for assertions to verify that timeoutExpired() triggers expected fallback behavior. This test currently lacks any assertion, so it does not verify meaningful behavior. Adding assertions on observable outcomes will improve test quality.

## `DontScheduleFallbackOnTimeoutNoCob` (line 79)
**Issue Type:** no_assertion
**Problem:** The test calls timeoutExpired() on a query without a callback but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that timeoutExpired() does not schedule fallback when no callback is provided, by checking query state or side effects.
**Fix:** This fix adds a placeholder for assertions to verify that timeoutExpired() does not schedule fallback when no callback is provided. Without assertions, the test only verifies no crash, which is insufficient.

## `ScheduleFallbackOnFailureDnscrNoCob` (line 98)
**Issue Type:** no_assertion
**Problem:** The test calls fail() on a query without a callback but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that fail() triggers expected fallback or error handling behavior even when no callback is provided.
**Fix:** The fix adds a placeholder for assertions to verify that fail() triggers expected fallback or error handling behavior even without a callback. The original test lacks assertions and only verifies no crash.

 ---
_This diff was auto-generated by the Test Quality Analysis Pipeline_

## `ScheduleFallbackOnTimeout` - no_assertion
**Problem:** The test calls timeoutExpired() on a query but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that timeoutExpired() triggers the expected fallback scheduling behavior, such as invoking a callback method or changing query state.
**Solution:** The fix adds a placeholder for assertions to verify that timeoutExpired() triggers expected fallback behavior. This test currently lacks any assertion, so it does not verify meaningful behavior. Adding assertions on observable outcomes will improve test quality.

**Before:**
```
TEST_F(CAresResolverTest, ScheduleFallbackOnTimeout) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto cb = std::make_unique<MockResolutionCallback>();
  auto query =
      std::make_unique<MockQueryWithCob>(resolver.get(),
                                         CAresResolver::RecordType::kTXT,
                                         name,
                                         true,
                                         std::move(te),
                                         nullptr,
                                         std::move(teContext),
                                         cb.get());
  query->timeoutExpired();
}
```

**After:**
```
TEST_F(CAresResolverTest, ScheduleFallbackOnTimeout) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto cb = std::make_unique<MockResolutionCallback>();
  auto query =
      std::make_unique<MockQueryWithCob>(resolver.get(),
                                         CAresResolver::RecordType::kTXT,
                                         name,
                                         true,
                                         std::move(te),
                                         nullptr,
                                         std::move(teContext),
                                         cb.get());
  query->timeoutExpired();
  // Assert that the callback or fallback was triggered
  // For example, check if queryFinished() was called or callback state changed
  // Since MockResolutionCallback methods are empty, add a flag or mock method to verify
  SUCCEED(); // Placeholder until observable behavior is added
}
```

## `DontScheduleFallbackOnTimeoutNoCob` - no_assertion
**Problem:** The test calls timeoutExpired() on a query without a callback but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that timeoutExpired() does not schedule fallback when no callback is provided, by checking query state or side effects.
**Solution:** This fix adds a placeholder for assertions to verify that timeoutExpired() does not schedule fallback when no callback is provided. Without assertions, the test only verifies no crash, which is insufficient.

**Before:**
```
TEST_F(CAresResolverTest, DontScheduleFallbackOnTimeoutNoCob) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto cb = std::make_unique<MockResolutionCallback>();
  auto query =
      std::make_unique<MockQueryWithCob>(resolver.get(),
                                         CAresResolver::RecordType::kTXT,
                                         name,
                                         true,
                                         std::move(te),
                                         nullptr,
                                         std::move(teContext),
                                         nullptr);
  query->timeoutExpired();
}
```

**After:**
```
TEST_F(CAresResolverTest, DontScheduleFallbackOnTimeoutNoCob) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto query =
      std::make_unique<MockQueryWithCob>(resolver.get(),
                                         CAresResolver::RecordType::kTXT,
                                         name,
                                         true,
                                         std::move(te),
                                         nullptr,
                                         std::move(teContext),
                                         nullptr);
  query->timeoutExpired();
  // Assert that no fallback was scheduled, e.g., no callback invoked
  SUCCEED(); // Placeholder until observable behavior is added
}
```

## `ScheduleFallbackOnFailureDnscrNoCob` - no_assertion
**Problem:** The test calls fail() on a query without a callback but does not assert any observable outcome or state change, so it does not verify meaningful behavior.
**Behavior to Test:** Verify that fail() triggers expected fallback or error handling behavior even when no callback is provided.
**Solution:** The fix adds a placeholder for assertions to verify that fail() triggers expected fallback or error handling behavior even without a callback. The original test lacks assertions and only verifies no crash.

**Before:**
```
TEST_F(CAresResolverTest, ScheduleFallbackOnFailureDnscrNoCob) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto query = new MockQueryWithCob(resolver.get(),
                                    CAresResolver::RecordType::kTXT,
                                    name,
                                    true,
                                    std::move(te),
                                    nullptr,
                                    std::move(teContext));
  query->fail(static_cast<DNSResolver::ResolutionStatus>(1), "error");
}
```

**After:**
```
TEST_F(CAresResolverTest, ScheduleFallbackOnFailureDnscrNoCob) {
  TraceEvent te(TraceEventType::DnsResolution);
  auto query = new MockQueryWithCob(resolver.get(),
                                    CAresResolver::RecordType::kTXT,
                                    name,
                                    true,
                                    std::move(te),
                                    nullptr,
                                    std::move(teContext));
  query->fail(static_cast<DNSResolver::ResolutionStatus>(1), "error");
  // Assert that fallback or error handling occurred
  SUCCEED(); // Placeholder until observable behavior is added
}
```

Please follow the steps below to resolve this test quality issue:
1. Update the code in xplat/proxygen/lib/dns/test/CAresResolverTest.cpp
2. Run the test(s) ScheduleFallbackOnTimeout, DontScheduleFallbackOnTimeoutNoCob, ScheduleFallbackOnFailureDnscrNoCob
3. Ensure everything works

 ---
[Run artifacts on Manifold](https://www.internalfb.com/manifold/explorer/wearables_autodev_learnings/tree/autodev/T273883387/21096823)

Differential Revision: D112743180

fbshipit-source-id: e63a2ea099b92ab7f673d2706774d47750e911ea

Author: Rob Kelly

proxygen/lib/dns/test/CAresResolverTest.cpp

@@ -79,12 +79,14 @@ class MockQueryWithCob : public MockQuery {
 class MockResolutionCallback : public CAresResolver::ResolutionCallback {
  public:
   using CAresResolver::ResolutionCallback::ResolutionCallback;
-  void resolutionSuccess(
-      std::vector<DNSResolver::Answer> /*answers*/) noexcept override {
-  }
-  void resolutionError(
-      const folly::exception_wrapper& /*ew*/) noexcept override {
-  }
+  MOCK_METHOD(void,
+              resolutionSuccess,
+              (std::vector<DNSResolver::Answer>),
+              (noexcept, override));
+  MOCK_METHOD(void,
+              resolutionError,
+              (const folly::exception_wrapper&),
+              (noexcept, override));
 };
 
 TEST_F(CAresResolverTest, ScheduleFallbackOnTimeout) {
@@ -99,12 +101,21 @@ TEST_F(CAresResolverTest, ScheduleFallbackOnTimeout) {
                                          nullptr,
                                          std::move(teContext),
                                          cb.get());
+
+  EXPECT_CALL(*resolver, queryFinished());
+  folly::exception_wrapper capturedError;
+  EXPECT_CALL(*cb, resolutionError(_)).WillOnce(SaveArg<0>(&capturedError));
+
   query->timeoutExpired();
+
+  ASSERT_TRUE(capturedError);
+  auto* dnsException = capturedError.get_exception<DNSResolver::Exception>();
+  ASSERT_NE(dnsException, nullptr);
+  EXPECT_EQ(dnsException->status(), DNSResolver::TIMEOUT);
 }
 
 TEST_F(CAresResolverTest, DontScheduleFallbackOnTimeoutNoCob) {
   TraceEvent te(TraceEventType::DnsResolution);
-  auto cb = std::make_unique<MockResolutionCallback>();
   auto query =
       std::make_unique<MockQueryWithCob>(resolver.get(),
                                          CAresResolver::RecordType::kTXT,
@@ -114,13 +125,18 @@ TEST_F(CAresResolverTest, DontScheduleFallbackOnTimeoutNoCob) {
                                          nullptr,
                                          std::move(teContext),
                                          nullptr);
+
+  // queryFinished() is invoked unconditionally by timeoutExpired() so the
+  // c-ares channel refcount stays balanced, even when no callback is attached.
+  EXPECT_CALL(*resolver, queryFinished());
+
   query->timeoutExpired();
 }
 
 TEST_F(CAresResolverTest, ScheduleFallbackOnFailureDnscr) {
   TraceEvent te(TraceEventType::DnsResolution);
   TimeUtil tu;
-  auto cb = std::make_unique<MockResolutionCallback>();
+  auto cb = std::make_unique<NiceMock<MockResolutionCallback>>();
   auto query = new MockQueryWithCob(resolver.get(),
                                     CAresResolver::RecordType::kTXT,
                                     name,
@@ -141,6 +157,12 @@ TEST_F(CAresResolverTest, ScheduleFallbackOnFailureDnscrNoCob) {
                                     std::move(te),
                                     nullptr,
                                     std::move(teContext));
+
+  // When no callback is attached, fail() must skip queryFinished() — the
+  // callback-guarded branch owns the resolver-side bookkeeping. The query
+  // still self-deletes; a leak would surface under ASAN.
+  EXPECT_CALL(*resolver, queryFinished()).Times(0);
+
   query->fail(static_cast<DNSResolver::ResolutionStatus>(1), "error");
 }
 
@@ -352,7 +374,7 @@ TEST_F(CAresResolverTest, CheckForCNameSynchronousCallbackNoUAF) {
 
   TraceEvent te(TraceEventType::DnsResolution);
   TimeUtil timeUtil;
-  auto cb = std::make_unique<MockResolutionCallback>();
+  auto cb = std::make_unique<NiceMock<MockResolutionCallback>>();
 
   // Create a Query for an A record lookup with the name that matches
   // the question in our crafted CNAME response.