Merge pull request #71 from ewired/refactorurls

Streamline TestOption processing

Author: anerli

packages/magnitude-test/src/discovery/testDeclaration.ts

@@ -1,6 +1,6 @@
 import { TestDeclaration, TestOptions, TestFunction, TestGroupFunction } from './types';
-import { TestRegistry, processUrl } from './testRegistry';
-import { addProtocolIfMissing } from '@/util';
+import { TestRegistry } from './testRegistry';
+import { addProtocolIfMissing, processUrl } from '@/util';
 
 function testDecl(
     title: string,
@@ -32,7 +32,7 @@ function testDecl(
     };
 
     if (!combinedOptions.url) {
-        throw Error("URL must be provided either through (1) env var MAGNITUDE_TEST_URL, (2) via test.config, or (3) in group or test options");
+        throw Error("URL must be provided either through (1) env var MAGNITUDE_TEST_URL, (2) via magnitude.config.ts, or (3) in group or test options");
     }
 
     // Add the declared test function as a runnable to the registry 

packages/magnitude-test/src/discovery/testRegistry.ts

@@ -1,5 +1,6 @@
 import { TestOptions, TestGroup, MagnitudeConfig, TestFunction, RegisteredTest } from "./types";
 import { TestCompiler } from "@/compiler";
+import { processUrl } from "@/util";
 import cuid2 from "@paralleldrive/cuid2";
 import { pathToFileURL } from "node:url";
 
@@ -140,28 +141,12 @@ export class TestRegistry {
             url: process.env.MAGNITUDE_TEST_URL
         } : {};
 
-        //console.log("global options:", this.globalOptions)
-
-        //const configuredOptions = this.globalOptions;
-        const globalOptions = this.globalOptions.url ? {
-            url: processUrl(envOptions.url, this.globalOptions.url)
-        } : {};
-
-        const groupOptions = this.currentGroup?.options ? {
-            ...this.currentGroup.options,
-            url: processUrl(globalOptions.url, this.currentGroup.options.url)
-        } : {};
-
-
-        const combinedOptions = {
-            ...globalOptions,
+        return {
+            ...this.globalOptions,
             ...envOptions, // env options take precedence over global options
-            ...groupOptions
-        }
-
-        //console.log("combinedOptions:", combinedOptions)
-
-        return combinedOptions;
+            ...(this.currentGroup?.options ?? {}),
+            url: processUrl(envOptions.url, this.globalOptions.url, this.currentGroup?.options?.url)
+        };
     }
 
     async loadTestFile(absoluteFilePath: string, relativeFilePath: string): Promise<void> {
@@ -191,19 +176,4 @@ export class TestRegistry {
     }
 }
 
-export function processUrl(base: string | undefined, relative: string | undefined): string | undefined {
-    if (!relative) return base;
-    if (!base) return relative;
-    try {
-        return new URL(relative).toString(); // It's a full URL by itself
-    } catch {
-        try {
-            // Not a full URL on its own, try to combine with base
-            return new URL(relative, base).toString();
-        } catch (e) {
-            return relative;
-        }
-    }
-}
-
 export default TestRegistry;

packages/magnitude-test/src/util.ts

@@ -208,3 +208,24 @@ export const knownCostMap: Record<string, number[]> = {
     'gpt-4.1-nano': [0.10, 0.40],
     'gpt-4o': [3.75, 15.00],
 }
+
+export function processUrl(...urls: (string | undefined)[]): string | undefined {
+    if (urls.length === 0) return;
+    if (urls.length === 1) return urls[0];
+
+    const [base, relative, ...rest] = urls;
+    if (!relative) return processUrl(base, ...rest);
+    if (!base) return processUrl(relative, ...rest);
+
+    try {
+        return processUrl(new URL(relative).toString(), ...rest); // It's a full URL by itself
+    } catch {
+        try {
+            // Not a full URL on its own, try to combine with base
+            return processUrl(new URL(relative, base).toString(), ...rest);
+        } catch (_e) {
+            return processUrl(relative, ...rest);
+        }
+    }
+}
+