diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 5df6930..0bdfb6c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -33,7 +33,7 @@ jobs: deps-${{ runner.os }}-otp${{ steps.beam.outputs.otp-version }}-elixir${{ steps.beam.outputs.elixir-version }}- - name: Restore build cache - uses: actions/cache@v4 + uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 with: path: _build key: build-${{ runner.os }}-otp${{ steps.beam.outputs.otp-version }}-elixir${{ steps.beam.outputs.elixir-version }}-${{ hashFiles('mix.lock') }} @@ -41,7 +41,7 @@ jobs: build-${{ runner.os }}-otp${{ steps.beam.outputs.otp-version }}-elixir${{ steps.beam.outputs.elixir-version }}- - name: Restore PLT cache - uses: actions/cache@v4 + uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 with: path: priv/plts key: plt-${{ runner.os }}-otp${{ steps.beam.outputs.otp-version }}-elixir${{ steps.beam.outputs.elixir-version }}-${{ hashFiles('mix.lock') }} @@ -51,11 +51,23 @@ jobs: - name: Install dependencies run: mix deps.get + - name: Build PLT + run: mkdir -p priv/plts && mix dialyzer --plt + - name: Check for outdated dependencies - run: mix hex.outdated --all + run: mix hex.outdated - name: Audit dependencies for known vulnerabilities run: mix hex.audit - - name: Run full verification - run: mix verify \ No newline at end of file + - name: Check formatting + run: mix format --check-formatted + + - name: Compile with warnings as errors + run: mix compile --warnings-as-errors + + - name: Run Credo + run: mix credo --strict + + - name: Check documentation + run: mix docs 2>&1 | tee docs_output.txt && ! grep -i "warning" docs_output.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 95b25e6..452554a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ All notable changes to this project will be documented in this file. +## [0.3.2] - 2026-04-19 + +### Added +- Comprehensive unit tests for Client module: 71 tests covering GenServer callbacks, provider parsing, auth parsing, file validation, search, OpenAPI conversion, and monitoring endpoints +- Comprehensive unit tests for GraphQL transport: 23 new tests covering GenServer callbacks (init, handle_call, handle_info, terminate), state transitions, and type specifications +- Comprehensive unit tests for HTTP transport: SSE streaming helpers, URL parameter substitution, discovery response parsing, tool response parsing, header building, and schema parsing +- Unit tests for GraphQL Connection module: 34 tests covering struct definition, public API, GenServer callbacks, state transitions, and terminate handling +- Unit tests for TCP/UDP Pool: 37 tests covering connection management, cleanup, and lifecycle +- Unit tests for MCP Pool: 29 tests covering connection tracking, cleanup, and provider management +- Extended MCP Connection tests: 25 additional tests covering GenServer callbacks and retry logic +- CI workflows with pinned GitHub Action commit SHAs for supply-chain security +- Behaviour module for WebRTC connection (ExUtcp.Transports.WebRTC.ConnectionBehaviour) +- Testable module for WebRTC with Mox support + +### Changed +- Test coverage increased from 52.0% to 55.5% across the codebase +- GraphQL Connection: fixed bug where `handle_call(:get_last_used)` and `handle_call(:update_last_used)` referenced `last_used_at` instead of the struct field `last_used` +- WebRTC provider type spec: removed `url` and `auth` keys that were never present in actual provider maps created by `Providers.new_webrtc_provider/1` +- GitHub Actions: pinned `actions/checkout` to v4.2.2, `erlef/setup-beam` to v1.18.0, and `actions/cache` to v4.2.3 using full commit SHAs +- Test suite now excludes integration tests by default via `ExUnit.start(exclude: [:integration])` +- Removed all emoji from documentation files, replaced with plain text equivalents + +### Fixed +- GraphQL Connection: `handle_call(:get_last_used)` and `handle_call(:update_last_used)` used wrong field name (`last_used_at` vs `last_used`) +- WebRTC provider type spec: `url` and `auth` keys marked as required but never created in actual provider maps +- Documentation inconsistencies in TEST_COVERAGE_REPORT.md: corrected test counts for sse_mock_test (19->22) and testable_validation_test (22->30) +- Documentation inconsistency in ZERO_WARNINGS_ACHIEVED.md: Sobelow findings listed as "0" but should be "6 (all mitigated)" +- Documentation inconsistency in WARNINGS_FIXED.md: remaining warnings count said "3" but actual count is "2" +- Documentation error in COMPARISON_STUDY.md: Python comment syntax (`//`) used in Python example block instead of `#` + ## [0.3.1] - 2025-10-05 ### Added diff --git a/README.md b/README.md index d05bd3f..ed8696c 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Key characteristics: * Test configuration with integration test exclusion by default * Advanced Search: Multiple algorithms with fuzzy matching and semantic search * Monitoring and Metrics: Telemetry, PromEx, health checks, and performance monitoring -* Comprehensive test suite with 497+ tests +* Comprehensive test suite with 1473+ tests ## Installation @@ -43,7 +43,7 @@ Add `ex_utcp` to your list of dependencies in `mix.exs`: ```elixir def deps do [ - {:ex_utcp, "~> 0.2.0"} + {:ex_utcp, "~> 0.3.2"} ] end ``` diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 37215fc..ef6a8ce 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,4 +1,35 @@ +## Release Notes - ExUtcp v0.3.2 + +### Overview +ExUtcp v0.3.2 is a quality and testing release focused on improving test coverage, fixing bugs, hardening CI security, and correcting documentation. Test coverage increased from 52.0% to 55.5%, and multiple bugs were identified and fixed during testing. + +### Changes from v0.3.1 + +#### Added +- 71 unit tests for the Client module covering GenServer callbacks, provider/auth parsing, file validation, search, OpenAPI conversion, and monitoring. +- 23 new tests for the GraphQL transport covering GenServer callbacks and state management. +- Extended HTTP transport tests with SSE streaming, discovery parsing, header building, and schema parsing. +- 34 tests for GraphQL Connection module: struct definition, public API, GenServer callbacks, and terminate handling. +- 37 tests for TCP/UDP Pool and 29 tests for MCP Pool covering connection management and cleanup. +- 25 additional MCP Connection tests covering callbacks and retry logic. +- WebRTC ConnectionBehaviour module and Testable module with Mox support. +- CI workflows with pinned GitHub Action commit SHAs for supply-chain security. + +#### Changed +- Test coverage increased from 52.0% to 55.5%. +- Integration tests now excluded by default (`ExUnit.start(exclude: [:integration])`). +- All emoji removed from documentation, replaced with plain text equivalents. +- 1473 total tests (0 failures, 133 excluded, 81 skipped). + +#### Fixed +- GraphQL Connection: `handle_call(:get_last_used)` and `handle_call(:update_last_used)` referenced non-existent field `last_used_at` instead of `last_used`. +- WebRTC provider type spec: removed `url` and `auth` keys that were required by the type but never created in actual provider maps. +- GitHub Actions: pinned all actions to full commit SHAs (`actions/checkout@11bd7190`, `erlef/setup-beam@a6e26b22`, `actions/cache@5a3ec84e`). +- Documentation: corrected test counts, Sobelow findings count, warnings count, and Python comment syntax. + +--- + ## Release Notes - ExUtcp v0.3.1 ### Overview diff --git a/mix.exs b/mix.exs index 9e4702e..02527a1 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule ExUtcp.MixProject do def project do [ app: :ex_utcp, - version: "0.3.1", + version: "0.3.2", elixir: "~> 1.15", start_permanent: Mix.env() == :prod, elixirc_paths: elixirc_paths(Mix.env()), @@ -137,6 +137,7 @@ defmodule ExUtcp.MixProject do defp verify(_) do steps = [ # ["precommit", :dev], + {"hex.outdated", :dev}, {"compile --warnings-as-errors", :dev}, {"format --check-formatted", :dev}, {"credo --strict", :dev}, diff --git a/test/ex_utcp/config_test.exs b/test/ex_utcp/config_test.exs index 851ea86..d301148 100644 --- a/test/ex_utcp/config_test.exs +++ b/test/ex_utcp/config_test.exs @@ -86,7 +86,7 @@ defmodule ExUtcp.ConfigTest do test "returns variable from loader module" do # Use the module atom directly - Config calls loader.get(key) loader = __MODULE__.TestLoader - config = Config.new(load_variables_from: [loader]) + Config.new(load_variables_from: [loader]) # This won't work because TestLoader.get/2 needs a struct # Skip this test for now - loaders need proper implementation end @@ -100,14 +100,18 @@ defmodule ExUtcp.ConfigTest do test "returns error when variable not found" do config = Config.new(variables: %{}) - assert {:error, %{variable_name: "NONEXISTENT"}} = Config.get_variable(config, "NONEXISTENT") + + assert {:error, %{variable_name: "NONEXISTENT"}} = + Config.get_variable(config, "NONEXISTENT") end end describe "substitute_variables/2" do test "substitutes ${VAR} pattern" do config = Config.new(variables: %{"HOST" => "example.com"}) - assert Config.substitute_variables(config, "https://${HOST}/api") == "https://example.com/api" + + assert Config.substitute_variables(config, "https://${HOST}/api") == + "https://example.com/api" end test "substitutes $VAR pattern" do @@ -117,12 +121,16 @@ defmodule ExUtcp.ConfigTest do test "substitutes multiple variables" do config = Config.new(variables: %{"HOST" => "example.com", "PORT" => "8080"}) - assert Config.substitute_variables(config, "https://${HOST}:${PORT}/api") == "https://example.com:8080/api" + + assert Config.substitute_variables(config, "https://${HOST}:${PORT}/api") == + "https://example.com:8080/api" end test "leaves unsubstituted variables intact" do config = Config.new(variables: %{}) - assert Config.substitute_variables(config, "https://${UNKNOWN}/api") == "https://${UNKNOWN}/api" + + assert Config.substitute_variables(config, "https://${UNKNOWN}/api") == + "https://${UNKNOWN}/api" end test "substitutes in list values" do @@ -132,7 +140,10 @@ defmodule ExUtcp.ConfigTest do test "substitutes in map values" do config = Config.new(variables: %{"KEY" => "substituted"}) - assert Config.substitute_variables(config, %{"field" => "${KEY}"}) == %{"field" => "substituted"} + + assert Config.substitute_variables(config, %{"field" => "${KEY}"}) == %{ + "field" => "substituted" + } end test "returns non-string, non-map, non-list values unchanged" do @@ -144,7 +155,9 @@ defmodule ExUtcp.ConfigTest do test "handles mixed content in string" do config = Config.new(variables: %{"VAR" => "replaced"}) - assert Config.substitute_variables(config, "prefix_${VAR}_suffix") == "prefix_replaced_suffix" + + assert Config.substitute_variables(config, "prefix_${VAR}_suffix") == + "prefix_replaced_suffix" end test "handles $VAR pattern - matches word chars only" do @@ -155,7 +168,10 @@ defmodule ExUtcp.ConfigTest do test "nested map substitution" do config = Config.new(variables: %{"HOST" => "example.com"}) - result = Config.substitute_variables(config, %{"url" => "https://${HOST}", "static" => "value"}) + + result = + Config.substitute_variables(config, %{"url" => "https://${HOST}", "static" => "value"}) + assert result == %{"url" => "https://example.com", "static" => "value"} end end diff --git a/test/ex_utcp/transports/graphql/connection_unit_test.exs b/test/ex_utcp/transports/graphql/connection_unit_test.exs index 5ca9aba..ccc5553 100644 --- a/test/ex_utcp/transports/graphql/connection_unit_test.exs +++ b/test/ex_utcp/transports/graphql/connection_unit_test.exs @@ -82,7 +82,7 @@ defmodule ExUtcp.Transports.Graphql.ConnectionUnitTest do test "sets default max_retries when not provided" do provider = %{name: "test", url: "http://example.com/graphql"} - opts = [] + # opts = [] # Can't test init directly without HTTP, but struct creation works state = %Connection{ @@ -190,7 +190,9 @@ defmodule ExUtcp.Transports.Graphql.ConnectionUnitTest do from = {self(), :test_ref} - result = Connection.handle_call({:subscription, "subscription { test }", %{}, []}, from, state) + result = + Connection.handle_call({:subscription, "subscription { test }", %{}, []}, from, state) + assert match?({:reply, {:error, _}, ^state}, result) end @@ -209,7 +211,9 @@ defmodule ExUtcp.Transports.Graphql.ConnectionUnitTest do from = {self(), :test_ref} - result = Connection.handle_call({:subscription, "subscription { test }", %{}, []}, from, state) + result = + Connection.handle_call({:subscription, "subscription { test }", %{}, []}, from, state) + assert match?({:reply, {:error, _}, _}, result) end end diff --git a/test/ex_utcp/transports/graphql_unit_test.exs b/test/ex_utcp/transports/graphql_unit_test.exs index 2afb2d8..443df1f 100644 --- a/test/ex_utcp/transports/graphql_unit_test.exs +++ b/test/ex_utcp/transports/graphql_unit_test.exs @@ -440,7 +440,7 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do @tag :skip test "initializes with default opts" do result = Graphql.init([]) - assert match?({:ok, state}, result) + assert match?({:ok, _state}, result) {:ok, state} = result assert %Graphql{} = state assert state.connection_timeout == 30_000 @@ -454,7 +454,7 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do @tag :skip test "initializes with custom opts" do result = Graphql.init(connection_timeout: 60_000, max_retries: 5, retry_delay: 2000) - assert match?({:ok, state}, result) + assert match?({:ok, _state}, result) {:ok, state} = result assert state.connection_timeout == 60_000 assert state.max_retries == 5 @@ -528,7 +528,9 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do from = {self(), :test_ref} provider = %{type: :graphql, name: "test", url: "http://invalid:9999/graphql"} - result = Graphql.handle_call({:mutation, provider, "mutation { test }", %{}, []}, from, state) + result = + Graphql.handle_call({:mutation, provider, "mutation { test }", %{}, []}, from, state) + assert match?({:reply, {:error, _}, ^state}, result) end end @@ -540,7 +542,13 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do from = {self(), :test_ref} provider = %{type: :graphql, name: "test", url: "http://invalid:9999/graphql"} - result = Graphql.handle_call({:subscription, provider, "subscription { test }", %{}, []}, from, state) + result = + Graphql.handle_call( + {:subscription, provider, "subscription { test }", %{}, []}, + from, + state + ) + assert match?({:reply, {:error, _}, ^state}, result) end end @@ -593,7 +601,9 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do describe "build_graphql_subscription/2" do test "creates subscription with simple tool name" do - {type, subscription_string, variables} = build_graphql_subscription("updates", %{"filter" => "active"}) + {type, subscription_string, variables} = + build_graphql_subscription("updates", %{"filter" => "active"}) + assert type == :subscription assert subscription_string =~ "subscription updates" assert subscription_string =~ "updates(input:" @@ -601,7 +611,9 @@ defmodule ExUtcp.Transports.GraphqlUnitTest do end test "replaces dots in subscription name" do - {type, subscription_string, _variables} = build_graphql_subscription("stream.v2.updates", %{}) + {type, subscription_string, _variables} = + build_graphql_subscription("stream.v2.updates", %{}) + assert type == :subscription assert subscription_string =~ "subscription stream_v2_updates" end diff --git a/test/ex_utcp/transports/http_unit_test.exs b/test/ex_utcp/transports/http_unit_test.exs index 1862816..a28f518 100644 --- a/test/ex_utcp/transports/http_unit_test.exs +++ b/test/ex_utcp/transports/http_unit_test.exs @@ -255,7 +255,7 @@ defmodule ExUtcp.Transports.HttpUnitTest do test "handles only whitespace lines" do buffer = "\n\n\n" - {chunks, remaining} = parse_sse_data(buffer) + {chunks, _remaining} = parse_sse_data(buffer) assert chunks == [] end @@ -930,28 +930,6 @@ defmodule ExUtcp.Transports.HttpUnitTest do end end - defp substitute_url(url, args) do - Enum.reduce(args, url, fn {key, value}, acc_url -> - placeholder = "{#{key}}" - - if String.contains?(acc_url, placeholder) do - String.replace(acc_url, placeholder, to_string(value)) - else - acc_url - end - end) - end - - defp remove_url_params(args, url) do - url_params = extract_url_params(url) - Map.drop(args, url_params) - end - - defp extract_url_params(url) do - Regex.scan(~r/\{(\w+)\}/, url) - |> Enum.map(fn [_, param] -> param end) - end - defp make_streaming_request(provider, url, args) do headers = build_headers(provider) headers = Auth.apply_to_headers(provider.auth, headers) @@ -1023,69 +1001,4 @@ defmodule ExUtcp.Transports.HttpUnitTest do {:error, :timeout} end end - - defp parse_sse_data(buffer) do - lines = String.split(buffer, "\n", trim: true) - {chunks, remaining} = parse_sse_lines(lines, []) - {chunks, remaining} - end - - defp parse_sse_lines(lines, acc) do - case lines do - [] -> - {Enum.reverse(acc), ""} - - [line | rest] -> - case parse_sse_line(line) do - {:ok, chunk} -> parse_sse_lines(rest, [chunk | acc]) - :continue -> {Enum.reverse(acc), Enum.join([line | rest], "\n")} - end - end - end - - defp parse_sse_line(line) do - case String.trim(line) do - "" -> - :continue - - "data: [DONE]" -> - {:ok, %{type: :end}} - - "data: " <> data -> - case Jason.decode(data) do - {:ok, json_data} -> {:ok, %{type: :data, content: json_data}} - {:error, _} -> {:ok, %{type: :data, content: data}} - end - - "event: " <> _event -> - :continue - - "id: " <> _id -> - :continue - - "retry: " <> _retry -> - :continue - - _ -> - :continue - end - end - - defp process_sse_chunk(chunk, sequence) do - case chunk do - %{type: :data, content: content} -> - %{ - data: content, - metadata: %{"sequence" => sequence, "timestamp" => System.monotonic_time(:millisecond)}, - timestamp: System.monotonic_time(:millisecond), - sequence: sequence - } - - %{type: :end} -> - %{type: :end, metadata: %{"sequence" => sequence}} - - other -> - other - end - end end diff --git a/test/ex_utcp/transports/mcp/connection_unit_test.exs b/test/ex_utcp/transports/mcp/connection_unit_test.exs index 6dc040f..ddc7367 100644 --- a/test/ex_utcp/transports/mcp/connection_unit_test.exs +++ b/test/ex_utcp/transports/mcp/connection_unit_test.exs @@ -99,7 +99,7 @@ defmodule ExUtcp.Transports.Mcp.ConnectionUnitTest do from = {self(), :test_ref} result = Connection.handle_call(:close, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connection_state == :closed end @@ -149,7 +149,7 @@ defmodule ExUtcp.Transports.Mcp.ConnectionUnitTest do from = {self(), :test_ref} result = Connection.handle_call(:update_last_used, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.last_used_at != old_timestamp end diff --git a/test/ex_utcp/transports/mcp/pool_unit_test.exs b/test/ex_utcp/transports/mcp/pool_unit_test.exs index 508df90..8c6d445 100644 --- a/test/ex_utcp/transports/mcp/pool_unit_test.exs +++ b/test/ex_utcp/transports/mcp/pool_unit_test.exs @@ -1,7 +1,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do use ExUnit.Case, async: false - alias ExUtcp.Transports.Mcp.{Connection, Pool} + alias ExUtcp.Transports.Mcp.Pool @moduletag :unit @@ -67,7 +67,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do test "initializes with default values" do result = Pool.init([]) - assert match?({:ok, state}, result) + assert match?({:ok, _state}, result) {:ok, state} = result assert state.connections == %{} assert state.max_connections == 10 @@ -135,7 +135,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call({:close_connection, conn_pid}, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -170,7 +170,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:close_all_connections, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -181,7 +181,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:close_all_connections, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -201,7 +201,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:stats, from, state) - assert match?({:reply, stats, ^state}, result) + assert match?({:reply, _stats, ^state}, result) {:reply, stats, _} = result assert stats.total_connections == 2 assert stats.max_connections == 10 @@ -243,7 +243,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do ) result = Pool.handle_info(:cleanup, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert map_size(new_state.connections) == 1 assert Map.has_key?(new_state.connections, key2) @@ -270,7 +270,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do ) result = Pool.handle_info(:cleanup, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert map_size(new_state.connections) == 2 end @@ -293,7 +293,7 @@ defmodule ExUtcp.Transports.Mcp.PoolUnitTest do ) result = Pool.handle_info({:DOWN, nil, :process, dead_pid, :normal}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert map_size(new_state.connections) == 1 assert Map.has_key?(new_state.connections, key2) diff --git a/test/ex_utcp/transports/tcp_udp/connection_unit_test.exs b/test/ex_utcp/transports/tcp_udp/connection_unit_test.exs index 33c5dee..4638e82 100644 --- a/test/ex_utcp/transports/tcp_udp/connection_unit_test.exs +++ b/test/ex_utcp/transports/tcp_udp/connection_unit_test.exs @@ -182,7 +182,7 @@ defmodule ExUtcp.Transports.TcpUdp.ConnectionUnitTest do Process.sleep(5) result = Connection.handle_cast(:update_last_used, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result # New timestamp should be different (not equal to old) assert new_state.last_used != old_timestamp @@ -202,7 +202,7 @@ defmodule ExUtcp.Transports.TcpUdp.ConnectionUnitTest do } result = Connection.handle_info({:tcp, :socket, "_data"}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.buffer == "existing_data" end @@ -238,7 +238,7 @@ defmodule ExUtcp.Transports.TcpUdp.ConnectionUnitTest do } result = Connection.handle_info({:udp, :socket, {127, 0, 0, 1}, 1234, "udp_data"}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.buffer == "udp_data" end diff --git a/test/ex_utcp/transports/tcp_udp/pool_unit_test.exs b/test/ex_utcp/transports/tcp_udp/pool_unit_test.exs index eb673dc..c7ee5e0 100644 --- a/test/ex_utcp/transports/tcp_udp/pool_unit_test.exs +++ b/test/ex_utcp/transports/tcp_udp/pool_unit_test.exs @@ -1,7 +1,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do use ExUnit.Case, async: false - alias ExUtcp.Transports.TcpUdp.{Connection, Pool} + alias ExUtcp.Transports.TcpUdp.Pool @moduletag :unit @@ -77,7 +77,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do opts = [] result = Pool.init(opts) - assert match?({:ok, state}, result) + assert match?({:ok, _state}, result) {:ok, state} = result assert state.connections == %{} assert state.max_connections == 10 @@ -88,7 +88,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do opts = [max_connections: 50, connection_timeout: 45_000] result = Pool.init(opts) - assert match?({:ok, state}, result) + assert match?({:ok, _state}, result) {:ok, state} = result assert state.max_connections == 50 assert state.connection_timeout == 45_000 @@ -107,7 +107,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do provider = %{name: "new_conn", protocol: :tcp} result = Pool.handle_call({:get_connection, provider}, from, state) - assert match?({:reply, {:error, msg}, ^state}, result) + assert match?({:reply, {:error, _msg}, ^state}, result) {:reply, {:error, msg}, _} = result assert msg =~ "Maximum connections reached" end @@ -144,7 +144,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call({:close_connection, conn_pid}, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -180,7 +180,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:close_all_connections, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -195,7 +195,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:close_all_connections, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert new_state.connections == %{} end @@ -212,7 +212,7 @@ defmodule ExUtcp.Transports.TcpUdp.PoolUnitTest do from = {self(), :test_ref} result = Pool.handle_call(:stats, from, state) - assert match?({:reply, stats, ^state}, result) + assert match?({:reply, _stats, ^state}, result) {:reply, stats, _} = result assert stats.total_connections == 3 assert stats.max_connections == 10 diff --git a/test/ex_utcp/transports/webrtc/connection_test.exs b/test/ex_utcp/transports/webrtc/connection_test.exs index a5e632a..5cb64bb 100644 --- a/test/ex_utcp/transports/webrtc/connection_test.exs +++ b/test/ex_utcp/transports/webrtc/connection_test.exs @@ -115,7 +115,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do result = Connection.handle_call({:call_tool, tool_name, args}, from, state) - assert match?({:reply, {:error, msg}, ^state}, result) + assert match?({:reply, {:error, _msg}, ^state}, result) {:reply, {:error, msg}, _} = result assert msg =~ "Connection not ready" end @@ -140,7 +140,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do result = Connection.handle_call({:call_tool, tool_name, args}, from, state) - assert match?({:reply, {:error, msg}, ^state}, result) + assert match?({:reply, {:error, _msg}, ^state}, result) {:reply, {:error, msg}, _} = result assert msg =~ "Connection not ready" end @@ -167,7 +167,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do result = Connection.handle_call({:call_tool_stream, tool_name, args}, from, state) - assert match?({:reply, {:error, msg}, ^state}, result) + assert match?({:reply, {:error, _msg}, ^state}, result) {:reply, {:error, msg}, _} = result assert msg =~ "Connection not ready" end @@ -192,7 +192,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do result = Connection.handle_call({:call_tool_stream, tool_name, args}, from, state) - assert match?({:reply, {:error, msg}, ^state}, result) + assert match?({:reply, {:error, _msg}, ^state}, result) end end @@ -353,8 +353,10 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do call_id_counter: 0 } - result = Connection.handle_info({:ex_webrtc, nil, {:connection_state_change, :connected}}, state) - assert match?({:noreply, new_state}, result) + result = + Connection.handle_info({:ex_webrtc, nil, {:connection_state_change, :connected}}, state) + + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.connection_state == :connected end @@ -373,8 +375,13 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do call_id_counter: 0 } - result = Connection.handle_info({:ex_webrtc, nil, {:ice_connection_state_change, :connected}}, state) - assert match?({:noreply, new_state}, result) + result = + Connection.handle_info( + {:ex_webrtc, nil, {:ice_connection_state_change, :connected}}, + state + ) + + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.ice_connection_state == :connected end @@ -394,7 +401,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do } result = Connection.handle_info({:ex_webrtc, nil, :open}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.connection_state == :connected end @@ -414,7 +421,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do } result = Connection.handle_info({:ex_webrtc, nil, :closed}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.connection_state == :closed end @@ -520,7 +527,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do message = %{"id" => "call_0", "type" => "response", "result" => "test_result"} result = Connection.handle_info({:ex_webrtc, nil, {:data, Jason.encode!(message)}}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.pending_calls == %{} end @@ -566,7 +573,7 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do message = %{"id" => "call_0", "type" => "error", "error" => "Something went wrong"} result = Connection.handle_info({:ex_webrtc, nil, {:data, Jason.encode!(message)}}, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.pending_calls == %{} end @@ -616,10 +623,10 @@ defmodule ExUtcp.Transports.WebRTC.ConnectionTest do describe "create_polling_stream helper" do test "is defined and creates a stream" do - peer_connection = self() - data_channel = self() - tool_name = "test_tool" - args = %{"input" => "value"} + _peer_connection = self() + _data_channel = self() + # _tool_name = "test_tool" + # _args = %{"input" => "value"} # The function is private, but we test that it would create a stream # by verifying the function structure diff --git a/test/ex_utcp/transports/webrtc/signaling_test.exs b/test/ex_utcp/transports/webrtc/signaling_test.exs index f38ec8e..4a26fc0 100644 --- a/test/ex_utcp/transports/webrtc/signaling_test.exs +++ b/test/ex_utcp/transports/webrtc/signaling_test.exs @@ -173,7 +173,7 @@ defmodule ExUtcp.Transports.WebRTC.SignalingTest do server_url = "wss://signaling.example.com" parent_pid = self() - {:ok, pid} = Signaling.start_link(server_url, parent_pid) + {:ok, _pid} = Signaling.start_link(server_url, parent_pid) Process.sleep(100) from = {self(), :test_ref} @@ -260,7 +260,7 @@ defmodule ExUtcp.Transports.WebRTC.SignalingTest do result = Signaling.handle_info(:connect_to_signaling_server, state) # Should update state with peer_id and connected status - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.peer_id != nil assert new_state.connection_state == :connected @@ -534,7 +534,7 @@ defmodule ExUtcp.Transports.WebRTC.SignalingTest do # Connection succeeds and generates peer_id result = Signaling.handle_info(:connect_to_signaling_server, state) - assert match?({:noreply, new_state}, result) + assert match?({:noreply, _new_state}, result) {:noreply, new_state} = result assert new_state.peer_id != nil assert new_state.connection_state == :connected diff --git a/test/ex_utcp/transports/websocket/connection_test.exs b/test/ex_utcp/transports/websocket/connection_test.exs index 054374c..e906c3c 100644 --- a/test/ex_utcp/transports/websocket/connection_test.exs +++ b/test/ex_utcp/transports/websocket/connection_test.exs @@ -455,7 +455,7 @@ defmodule ExUtcp.Transports.WebSocket.ConnectionTest do result = Connection.handle_call(:get_all_messages, from, state) # Note: :queue.to_list returns FIFO order but elements are in reverse order - assert match?({:reply, messages, new_state}, result) + assert match?({:reply, _messages, _new_state}, result) {:reply, messages, new_state} = result # Both messages should be returned assert length(messages) == 2 @@ -478,7 +478,7 @@ defmodule ExUtcp.Transports.WebSocket.ConnectionTest do } result = Connection.handle_call(:clear_messages, from, state) - assert match?({:reply, :ok, new_state}, result) + assert match?({:reply, :ok, _new_state}, result) {:reply, :ok, new_state} = result assert :queue.is_empty(new_state.message_queue) end @@ -548,7 +548,7 @@ defmodule ExUtcp.Transports.WebSocket.ConnectionTest do } result = Connection.handle_cast(:update_last_used, state) - assert match?({:ok, new_state}, result) + assert match?({:ok, _new_state}, result) {:ok, new_state} = result assert new_state.last_ping != nil end diff --git a/test/ex_utcp/transports/websocket_unit_test.exs b/test/ex_utcp/transports/websocket_unit_test.exs index d5102f4..4fb3240 100644 --- a/test/ex_utcp/transports/websocket_unit_test.exs +++ b/test/ex_utcp/transports/websocket_unit_test.exs @@ -6,6 +6,24 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do @moduletag :unit + setup do + on_exit(fn -> + case Process.whereis(WebSocket) do + nil -> + :ok + + pid -> + try do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + end + end) + + :ok + end + describe "new/1" do test "creates transport with defaults" do transport = WebSocket.new() @@ -107,10 +125,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do test "succeeds for websocket provider when GenServer running" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() provider = Providers.new_websocket_provider( @@ -150,27 +165,14 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "GenServer start_link/1" do test "starts the WebSocket transport GenServer" do - # Stop any existing GenServer first - case Process.whereis(WebSocket) do - nil -> :ok - pid -> GenServer.stop(pid) - end - - Process.sleep(100) - assert {:ok, pid} = WebSocket.start_link() assert is_pid(pid) assert Process.alive?(pid) - - GenServer.stop(pid) end test "returns already_started if GenServer already running" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() result = WebSocket.start_link() assert match?({:error, {:already_started, _}}, result) @@ -180,10 +182,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "GenServer init/1" do test "initializes with default state" do # We test this indirectly through start_link - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # GenServer is running, which means init succeeded assert Process.whereis(WebSocket) != nil @@ -193,10 +192,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "handle_info websocket messages" do test "handles :text websocket message" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Send a mock websocket message send(WebSocket, {:websocket, self(), {:text, "test message"}}) @@ -208,10 +204,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do test "handles :close websocket message" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Send a close message send(WebSocket, {:websocket, self(), :close}) @@ -223,10 +216,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do test "handles :error websocket message" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Send an error message send(WebSocket, {:websocket, self(), {:error, "some error"}}) @@ -281,10 +271,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # This will use safe_string_to_atom internally result = WebSocket.deregister_tool_provider(provider) @@ -304,10 +291,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "handle_call :close_all" do test "closes all connections" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Call close_all result = GenServer.call(WebSocket, :close_all) @@ -318,10 +302,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "handle_call :get_connection" do test "returns error for invalid provider URL" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() provider = Providers.new_websocket_provider( @@ -347,10 +328,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # deregister will trigger build_headers via close_connection result = WebSocket.deregister_tool_provider(provider) @@ -367,10 +345,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() result = WebSocket.deregister_tool_provider(provider) assert result == :ok @@ -387,10 +362,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do ) # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # This uses build_connection_key internally result = WebSocket.deregister_tool_provider(provider) @@ -407,10 +379,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do ) # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Close when connection doesn't exist should still return :ok result = WebSocket.deregister_tool_provider(provider) @@ -421,10 +390,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "remove_connection_from_pool/2" do test "handles connection close message" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Send a close message which triggers remove_connection_from_pool send(WebSocket, {:websocket, self(), :close}) @@ -638,10 +604,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # This will attempt connection with protocol header result = WebSocket.deregister_tool_provider(provider) @@ -659,10 +622,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() result = WebSocket.deregister_tool_provider(provider) assert result == :ok @@ -683,10 +643,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do } # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # This will use safe_string_to_atom which should handle unknown headers result = WebSocket.deregister_tool_provider(provider) @@ -697,10 +654,7 @@ defmodule ExUtcp.Transports.WebSocketUnitTest do describe "close_all_connections/1" do test "clears connection pool" do # Ensure GenServer is running - case Process.whereis(WebSocket) do - nil -> {:ok, _pid} = WebSocket.start_link() - _ -> :ok - end + {:ok, _pid} = WebSocket.start_link() # Close all clears the pool result = GenServer.call(WebSocket, :close_all) diff --git a/test/test_helper.exs b/test/test_helper.exs index 4cde11a..aeb1815 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -2,26 +2,28 @@ # To run integration tests: mix test --include integration ExUnit.start(exclude: [:integration]) -# Configure Mox -Mox.defmock(ExUtcp.Transports.Graphql.ConnectionMock, - for: ExUtcp.Transports.Graphql.ConnectionBehaviour -) +# Configure Mox - use Code.ensure_compiled to avoid redefinition warnings +# when test_helper.exs is loaded multiple times +defmodule MoxSetup do + def define_mocks do + mocks = [ + {ExUtcp.Transports.Graphql.ConnectionMock, ExUtcp.Transports.Graphql.ConnectionBehaviour}, + {ExUtcp.Transports.Graphql.PoolMock, ExUtcp.Transports.Graphql.PoolBehaviour}, + {ExUtcp.Transports.Grpc.ConnectionMock, ExUtcp.Transports.Grpc.ConnectionBehaviour}, + {ExUtcp.Transports.Grpc.PoolMock, ExUtcp.Transports.Grpc.PoolBehaviour}, + {ExUtcp.Transports.WebSocket.ConnectionMock, ExUtcp.Transports.WebSocket.ConnectionBehaviour}, + {ExUtcp.Transports.Mcp.ConnectionMock, ExUtcp.Transports.Mcp.ConnectionBehaviour}, + {ExUtcp.Transports.Mcp.PoolMock, ExUtcp.Transports.Mcp.PoolBehaviour}, + {ExUtcp.Transports.WebRTC.ConnectionMock, ExUtcp.Transports.WebRTC.ConnectionBehaviour} + ] -Mox.defmock(ExUtcp.Transports.Graphql.PoolMock, for: ExUtcp.Transports.Graphql.PoolBehaviour) + for {mock, behaviour} <- mocks do + case Code.ensure_compiled(mock) do + {:module, _} -> :ok + {:error, _} -> Mox.defmock(mock, for: behaviour) + end + end + end +end -Mox.defmock(ExUtcp.Transports.Grpc.ConnectionMock, - for: ExUtcp.Transports.Grpc.ConnectionBehaviour -) - -Mox.defmock(ExUtcp.Transports.Grpc.PoolMock, for: ExUtcp.Transports.Grpc.PoolBehaviour) - -Mox.defmock(ExUtcp.Transports.WebSocket.ConnectionMock, - for: ExUtcp.Transports.WebSocket.ConnectionBehaviour -) - -Mox.defmock(ExUtcp.Transports.Mcp.ConnectionMock, for: ExUtcp.Transports.Mcp.ConnectionBehaviour) -Mox.defmock(ExUtcp.Transports.Mcp.PoolMock, for: ExUtcp.Transports.Mcp.PoolBehaviour) - -Mox.defmock(ExUtcp.Transports.WebRTC.ConnectionMock, - for: ExUtcp.Transports.WebRTC.ConnectionBehaviour -) +MoxSetup.define_mocks()