From 65de67134381dd096ae81058b70fd6af1ed7fe8b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 13 Dec 2025 13:38:48 +0000 Subject: [PATCH 1/2] Refactor tests to mock Bible AI API and scraping calls Streamlined test execution by: - Mocking `GetPassageHTML` in `pkg/app/passage_test.go` and `pkg/app/devo_test.go` to prevent external network calls to BibleGateway during unit tests. - Updating `pkg/app/api_client_test.go` to conditionally run integration tests against the real Bible AI API only if `BIBLE_API_URL` is explicitly set and not equal to "https://example.com". Otherwise, it defaults to a safe mock environment. - Adding a `Fallback: Scrape` test case in `passage_test.go` to verify the fallback logic without triggering actual network requests. These changes ensure that standard "MR tests" are fast and isolated, while still allowing a "comprehensive eval" against the real API via environment configuration. --- pkg/app/api_client_test.go | 19 ++++++++-- pkg/app/devo_test.go | 15 ++++++++ pkg/app/passage_test.go | 77 +++++++++++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/pkg/app/api_client_test.go b/pkg/app/api_client_test.go index e2fac62..88fb4ef 100644 --- a/pkg/app/api_client_test.go +++ b/pkg/app/api_client_test.go @@ -1,15 +1,26 @@ package app import ( + "os" "testing" ) func TestSubmitQuery(t *testing.T) { t.Run("Success", func(t *testing.T) { - // Force cleanup of environment to ensure we test Secret Manager fallback - // This handles cases where the runner might have lingering env vars - defer SetEnv("BIBLE_API_URL", "https://example.com")() - defer SetEnv("BIBLE_API_KEY", "api_key")() + // Check if we should run integration test against real API + // If BIBLE_API_URL is set and not example.com, we assume integration test mode + realURL, hasURL := os.LookupEnv("BIBLE_API_URL") + if hasURL && realURL != "" && realURL != "https://example.com" { + t.Logf("Running integration test against real API: %s", realURL) + // Ensure we have a key + if _, hasKey := os.LookupEnv("BIBLE_API_KEY"); !hasKey { + t.Log("Warning: BIBLE_API_URL set but BIBLE_API_KEY missing. Test might fail.") + } + } else { + // Mock mode + defer SetEnv("BIBLE_API_URL", "https://example.com")() + defer SetEnv("BIBLE_API_KEY", "api_key")() + } ResetAPIConfigCache() diff --git a/pkg/app/devo_test.go b/pkg/app/devo_test.go index 0ad7fcb..e10fbad 100644 --- a/pkg/app/devo_test.go +++ b/pkg/app/devo_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "golang.org/x/net/html" + "github.com/julwrites/BotPlatform/pkg/def" "github.com/julwrites/ScriptureBot/pkg/utils" ) @@ -81,6 +83,19 @@ func TestGetDevotionalData(t *testing.T) { defer UnsetEnv("BIBLE_API_KEY")() ResetAPIConfigCache() + // Mock GetPassageHTML to prevent external calls during fallback + originalGetPassageHTML := GetPassageHTML + defer func() { GetPassageHTML = originalGetPassageHTML }() + + GetPassageHTML = func(ref, ver string) *html.Node { + return mockGetPassageHTML(` +
Genesis 1
+
+

Mock devotional content.

+
+ `) + } + var env def.SessionData env.Props = map[string]interface{}{"ResourcePath": "../../resource"} env.Res = GetDevotionalData(env, "DTMSV") diff --git a/pkg/app/passage_test.go b/pkg/app/passage_test.go index 21f7c21..1844a4c 100644 --- a/pkg/app/passage_test.go +++ b/pkg/app/passage_test.go @@ -1,16 +1,33 @@ package app import ( + "errors" "strings" "testing" + "golang.org/x/net/html" + "github.com/julwrites/BotPlatform/pkg/def" "github.com/julwrites/ScriptureBot/pkg/utils" ) +func mockGetPassageHTML(htmlStr string) *html.Node { + doc, _ := html.Parse(strings.NewReader(htmlStr)) + return doc +} + func TestGetReference(t *testing.T) { - doc := GetPassageHTML("gen 1", "NIV") + // Mock GetPassageHTML + originalGetPassageHTML := GetPassageHTML + defer func() { GetPassageHTML = originalGetPassageHTML }() + + GetPassageHTML = func(ref, ver string) *html.Node { + return mockGetPassageHTML(` +
Genesis 1
+ `) + } + doc := GetPassageHTML("gen 1", "NIV") ref := GetReference(doc) if ref != "Genesis 1" { @@ -19,6 +36,18 @@ func TestGetReference(t *testing.T) { } func TestGetPassage(t *testing.T) { + // Mock GetPassageHTML + originalGetPassageHTML := GetPassageHTML + defer func() { GetPassageHTML = originalGetPassageHTML }() + + GetPassageHTML = func(ref, ver string) *html.Node { + return mockGetPassageHTML(` +
+

In the beginning was the Word.

+
+ `) + } + doc := GetPassageHTML("john 8", "NIV") passage := GetPassage("John 8", doc, "NIV") @@ -100,6 +129,52 @@ func TestGetBiblePassage(t *testing.T) { t.Errorf("Expected failure message, got '%s'", env.Res.Message) } }) + + t.Run("Fallback: Scrape", func(t *testing.T) { + defer UnsetEnv("BIBLE_API_URL")() + defer UnsetEnv("BIBLE_API_KEY")() + ResetAPIConfigCache() + + // Mock GetPassageHTML for fallback + originalGetPassageHTML := GetPassageHTML + defer func() { GetPassageHTML = originalGetPassageHTML }() + + GetPassageHTML = func(ref, ver string) *html.Node { + return mockGetPassageHTML(` +
Genesis 1
+
+

In the beginning God created the heavens and the earth.

+
+ `) + } + + var env def.SessionData + env.Msg.Message = "gen 1" + var conf utils.UserConfig + conf.Version = "NIV" + env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf)) + + // Force API error by ensuring SubmitQuery fails or by not mocking it (without creds it fails) + // But TestGetBiblePassage above overrides SubmitQuery. We should ensure it's not overridden or fails. + // Since we deferred restore in TestGetBiblePassage main body, we need to handle it. + // Wait, the main TestGetBiblePassage body restores SubmitQuery at end. + // So here SubmitQuery is likely still the mock from previous subtests if run sequentially? + // No, t.Run subtests share the parent scope but deferred calls in parent happen after all subtests. + // But in "Success: Verify Request" we set SubmitQuery = MockSubmitQuery. + // We should restore it inside that subtest or assume it persists. + // The parent TestGetBiblePassage deferred restore. + // So SubmitQuery is currently the Mock. + // We want it to fail. + SubmitQuery = func(req QueryRequest, result interface{}) error { + return errors.New("forced api error") + } + + env = GetBiblePassage(env) + + if !strings.Contains(env.Res.Message, "In the beginning") { + t.Errorf("Expected fallback passage content, got '%s'", env.Res.Message) + } + }) } func TestParsePassageFromHtml(t *testing.T) { From 8ff26303ecd8f2defff22f3a8d73a1fb1c099585 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 13 Dec 2025 13:47:59 +0000 Subject: [PATCH 2/2] Mock external API calls in tests and document testing strategy - Created `docs/TESTING.md` to outline the hybrid testing strategy (standard mocks vs. conditional live integration). - Refactored `pkg/app/passage_test.go` and `pkg/app/devo_test.go` to mock `GetPassageHTML`, preventing external scraping during unit tests. - Updated `pkg/app/api_client_test.go` to support conditional live API testing when `BIBLE_API_URL` is configured. - Cleaned up test logic to ensure proper isolation of global variable mocks. --- docs/TESTING.md | 48 +++++++++++++++++++++++++++++++++++++++++ pkg/app/passage_test.go | 14 +++--------- 2 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 docs/TESTING.md diff --git a/docs/TESTING.md b/docs/TESTING.md new file mode 100644 index 0000000..b9b204e --- /dev/null +++ b/docs/TESTING.md @@ -0,0 +1,48 @@ +# Testing Strategy + +This project employs a hybrid testing strategy to ensure code quality while minimizing external dependencies and costs. + +## Test Categories + +### 1. Unit Tests (Standard) +* **Default Behavior:** By default, all tests run in "mock mode". +* **Goal:** Fast, reliable, and cost-free verification of logic. +* **Mechanism:** External services (Bible AI API, BibleGateway scraping) are mocked using function replacement (e.g., `SubmitQuery`, `GetPassageHTML`) or interface mocking. +* **Execution:** these tests are run automatically on every Pull Request (MR). + +### 2. Integration Tests (Live) +* **Conditional Behavior:** Specific tests are capable of switching to "live mode" when appropriate environment variables are detected. +* **Goal:** Verify that the application correctly interacts with real external services (Contract Testing) and that credentials/configurations are valid. +* **Execution:** These tests should be run on a scheduled basis (e.g., nightly or weekly) or manually when verifying infrastructure changes. + +## Live Tests & Configuration + +The following tests support live execution: + +### `TestSubmitQuery` +* **File:** `pkg/app/api_client_test.go` +* **Description:** Verifies connectivity to the Bible AI API. +* **Trigger:** + * `BIBLE_API_URL` is set AND + * `BIBLE_API_URL` is NOT `https://example.com` +* **Required Variables:** + * `BIBLE_API_URL`: The endpoint of the Bible AI API. + * `BIBLE_API_KEY`: A valid API key. +* **Rationale:** Ensures that the client code (request marshaling, auth headers) matches the actual API expectation and that the API is reachable. + +### `TestUserDatabaseIntegration` +* **File:** `pkg/app/database_integration_test.go` +* **Description:** Verifies Read/Write operations to Google Cloud Firestore/Datastore. +* **Trigger:** + * `GCLOUD_PROJECT_ID` is set. +* **Required Variables:** + * `GCLOUD_PROJECT_ID`: The Google Cloud Project ID. + * *Note:* Requires active Google Cloud credentials (e.g., `GOOGLE_APPLICATION_CREDENTIALS` or `gcloud auth`). +* **Rationale:** Verifies that database permissions and client initialization are correct, preventing runtime errors in production. Uses a specific test user ID (`test-integration-user-DO-NOT-DELETE`) to avoid affecting real user data. + +## Rationale for Strategy + +1. **Cost Reduction:** The Bible AI API may incur costs per call. Mocking prevents racking up bills during routine development. +2. **Speed:** Live calls are slow. Mocked tests run instantly. +3. **Reliability:** External services can be flaky. Mocked tests only fail if the code is broken. +4. **Verification:** We still need to know if the API changed or if our secrets are wrong. The conditional integration tests provide this safety net without the daily cost/latency penalty. diff --git a/pkg/app/passage_test.go b/pkg/app/passage_test.go index 1844a4c..e1dbc6b 100644 --- a/pkg/app/passage_test.go +++ b/pkg/app/passage_test.go @@ -154,17 +154,9 @@ func TestGetBiblePassage(t *testing.T) { conf.Version = "NIV" env = utils.SetUserConfig(env, utils.SerializeUserConfig(conf)) - // Force API error by ensuring SubmitQuery fails or by not mocking it (without creds it fails) - // But TestGetBiblePassage above overrides SubmitQuery. We should ensure it's not overridden or fails. - // Since we deferred restore in TestGetBiblePassage main body, we need to handle it. - // Wait, the main TestGetBiblePassage body restores SubmitQuery at end. - // So here SubmitQuery is likely still the mock from previous subtests if run sequentially? - // No, t.Run subtests share the parent scope but deferred calls in parent happen after all subtests. - // But in "Success: Verify Request" we set SubmitQuery = MockSubmitQuery. - // We should restore it inside that subtest or assume it persists. - // The parent TestGetBiblePassage deferred restore. - // So SubmitQuery is currently the Mock. - // We want it to fail. + // Override SubmitQuery to force failure + originalSubmitQuerySub := SubmitQuery + defer func() { SubmitQuery = originalSubmitQuerySub }() SubmitQuery = func(req QueryRequest, result interface{}) error { return errors.New("forced api error") }