diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8a209272..5f911571 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -104,8 +104,12 @@ jobs: - name: Run tests with coverage id: tests run: | - # Run tests with coverage - output=$(mix test --cover 2>&1 || true) + # Run tests with coverage. The exit code is captured rather than + # discarded: this step must fail the job when the suite is red. + set +e + output=$(mix test --cover 2>&1) + test_exit_code=$? + set -e echo "$output" > test_output.txt # Parse test results @@ -134,11 +138,12 @@ jobs: fi echo "success_rate=$success_rate" >> $GITHUB_OUTPUT - exit_code=$? - echo "exit_code=$exit_code" >> $GITHUB_OUTPUT - continue-on-error: true + # Fail the job on a red suite. Reporting steps below still run + # because they depend on this step's outputs via always(). + exit $test_exit_code - name: Generate coverage report + if: always() id: coverage run: | # Generate coverage report with GitHub format @@ -164,6 +169,7 @@ jobs: continue-on-error: true - name: Run Credo analysis + if: always() id: credo run: | # Run Credo and capture output @@ -201,6 +207,7 @@ jobs: continue-on-error: true - name: Run Dialyzer analysis + if: always() id: dialyzer run: | # Ensure PLT is built @@ -228,6 +235,7 @@ jobs: continue-on-error: true - name: Create test results summary + if: always() id: summary run: | # Calculate overall score @@ -368,3 +376,64 @@ jobs: path: pr-comment/ retention-days: 1 continue-on-error: true + + # Integration tests are excluded from the default `mix test` run by + # test/test_helper.exs, so until now they had never executed in CI at all. + # They run here as a separate job rather than by changing the default + # exclusion, so local `mix test` stays fast. + # + # This is a hard gate: a red integration run blocks the merge. Note that 30 of + # the tests `--only integration` selects are `@tag :skip`, so the gate covers + # the 35 that actually execute. + integration-test: + name: Integration Tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:15 + env: + POSTGRES_PASSWORD: postgres + POSTGRES_DB: wanderer_test + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Elixir/OTP + uses: erlef/setup-beam@v1 + with: + elixir-version: ${{ env.ELIXIR_VERSION }} + otp-version: ${{ env.OTP_VERSION }} + + - name: Cache Elixir dependencies + uses: actions/cache@v3 + with: + path: | + deps + _build + key: ${{ runner.os }}-mix-${{ hashFiles('**/mix.lock') }} + restore-keys: ${{ runner.os }}-mix- + + - name: Install Elixir dependencies + run: | + mix deps.get + mix deps.compile + + - name: Setup database + run: | + mix ecto.create + mix ecto.migrate + + - name: Run integration tests + # --only, not --include: --include would run the whole suite on top of + # the tagged tests, duplicating the main test job's ~10 minutes and + # re-reporting failures it already covers. + run: mix test --only integration diff --git a/lib/wanderer_app/api/map.ex b/lib/wanderer_app/api/map.ex index a30e44e4..95f144d0 100644 --- a/lib/wanderer_app/api/map.ex +++ b/lib/wanderer_app/api/map.ex @@ -34,7 +34,10 @@ defmodule WandererApp.Api.Map do repo(WandererApp.Repo) table("maps_v1") - migration_defaults scopes: "'{wormholes}'" + # This value is injected verbatim into generated migrations. It must be + # Elixir source for a list of strings: `'{wormholes}'` is a charlist, which + # generated a default of the character codes of the literal "{wormholes}". + migration_defaults scopes: ~s(["wormholes"]) end json_api do diff --git a/priv/repo/migrations/20260801200000_fix_maps_scopes_default.exs b/priv/repo/migrations/20260801200000_fix_maps_scopes_default.exs new file mode 100644 index 00000000..f7b93dbb --- /dev/null +++ b/priv/repo/migrations/20260801200000_fix_maps_scopes_default.exs @@ -0,0 +1,46 @@ +defmodule WandererApp.Repo.Migrations.FixMapsScopesDefault do + @moduledoc """ + Repairs the `maps_v1.scopes` column default. + + Migrations 20260331192521 and 20260406213852 declared the default as + `default: '{wormholes}'`. In Elixir a single-quoted literal is a charlist, + i.e. `[123, 119, 111, ...]`, so Ecto emitted an eleven-element text array of + the character codes of the literal string `{wormholes}` instead of the + one-element array `{wormholes}`. + + Any row inserted without an explicit `scopes` value therefore received + garbage that cannot be cast back to `{:array, :atom}`, and every subsequent + read of that row failed with `Ash.Error.Unknown` ("cannot load ... as type"). + + This migration sets the correct default and repairs rows already written with + the bad value. + """ + + use Ecto.Migration + + # Derived from the offending literal rather than transcribed by hand: a typo + # in a hardcoded list would make the WHERE clause match nothing and silently + # repair no rows. Both 20260331192521 and 20260406213852 used exactly this + # literal, so this single value catches rows written by either. + @bad_default Enum.map(~c"{wormholes}", &Integer.to_string/1) + + def up do + alter table(:maps_v1) do + modify :scopes, {:array, :text}, default: ["wormholes"] + end + + execute( + "UPDATE maps_v1 SET scopes = ARRAY['wormholes']::text[] WHERE scopes = ARRAY[#{Enum.map_join(@bad_default, ",", &"'#{&1}'")}]::text[]" + ) + end + + # Deliberately not a mirror image of up/0. Restoring the original charlist + # default would reintroduce the bug, so this drops the default instead, and + # rows already repaired stay repaired. Rolling back therefore leaves the + # schema in a different -- but correct -- state rather than the prior one. + def down do + alter table(:maps_v1) do + modify :scopes, {:array, :text}, default: nil + end + end +end diff --git a/priv/resource_snapshots/repo/maps_v1/20260801200000.json b/priv/resource_snapshots/repo/maps_v1/20260801200000.json new file mode 100644 index 00000000..1974941f --- /dev/null +++ b/priv/resource_snapshots/repo/maps_v1/20260801200000.json @@ -0,0 +1,277 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "name", + "type": "text" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "slug", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "description", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "personal_note", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "public_api_key", + "type": "text" + }, + { + "allow_nil?": true, + "default": "[]", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "hubs", + "type": [ + "array", + "text" + ] + }, + { + "allow_nil?": false, + "default": "\"wormholes\"", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "scope", + "type": "text" + }, + { + "allow_nil?": true, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "deleted", + "type": "boolean" + }, + { + "allow_nil?": true, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "only_tracked_characters", + "type": "boolean" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "options", + "type": "text" + }, + { + "allow_nil?": false, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "webhooks_enabled", + "type": "boolean" + }, + { + "allow_nil?": false, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "sse_enabled", + "type": "boolean" + }, + { + "allow_nil?": true, + "default": "[\"wormholes\"]", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "scopes", + "type": [ + "array", + "text" + ] + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "inserted_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "updated_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "maps_v1_owner_id_fkey", + "on_delete": null, + "on_update": null, + "primary_key?": true, + "schema": null, + "table": "character_v1" + }, + "scale": null, + "size": null, + "source": "owner_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "F00091B5D3467DFD9D0B5DD8E5A5590E581D1F4D51F0D6CD07413358DC1AF2B7", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "maps_v1_unique_public_api_key_index", + "keys": [ + { + "type": "atom", + "value": "public_api_key" + } + ], + "name": "unique_public_api_key", + "nils_distinct?": true, + "where": null + }, + { + "all_tenants?": false, + "base_filter": null, + "index_name": "maps_v1_unique_slug_index", + "keys": [ + { + "type": "atom", + "value": "slug" + } + ], + "name": "unique_slug", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.WandererApp.Repo", + "schema": null, + "table": "maps_v1" +} \ No newline at end of file diff --git a/test/integration/map/map_scope_filtering_test.exs b/test/integration/map/map_scope_filtering_test.exs index 176b885b..9ad35cd6 100644 --- a/test/integration/map/map_scope_filtering_test.exs +++ b/test/integration/map/map_scope_filtering_test.exs @@ -213,6 +213,17 @@ defmodule WandererApp.Map.MapScopeFilteringTest do to_solar_system_id: @ls_system_halmah }) + # These "jump_*" keys live in the global cache and are never invalidated + # once written -- CachedInfo.get_solar_system_jump/2 only rebuilds the index + # when a key is missing. MapScopesTest asserts on the same system IDs + # (30_000_001 / 30_000_002 / 30_000_100) expecting NO stargate, so leaving + # these behind made its "valid when no stargate exists" cases fail depending + # on whether this suite happened to run first. + on_exit(fn -> + WandererApp.Cache.delete(halenan_mili_key) + WandererApp.Cache.delete(halenan_halmah_key) + end) + :ok end diff --git a/test/unit/controllers/auth_controller_test.exs b/test/unit/controllers/auth_controller_test.exs index 0cfb73fe..94f668e6 100644 --- a/test/unit/controllers/auth_controller_test.exs +++ b/test/unit/controllers/auth_controller_test.exs @@ -5,7 +5,7 @@ defmodule WandererAppWeb.AuthControllerTest do describe "parameter validation and error handling" do test "callback/2 validates missing assigns" do - conn = build_conn() + conn = browser_conn() params = %{} # Should handle gracefully when required assigns are missing @@ -34,7 +34,7 @@ defmodule WandererAppWeb.AuthControllerTest do test "callback/2 handles malformed auth data gracefully" do # Test with minimal conn structure to exercise error paths # The callback/2 function will match the fallback clause and redirect - conn = build_conn() + conn = browser_conn() result = AuthController.callback(conn, %{}) @@ -46,7 +46,7 @@ defmodule WandererAppWeb.AuthControllerTest do test "callback/2 processes auth structure with missing fields" do # Test the fallback clause since auth structure is incomplete # Missing CharacterOwnerHash will cause pattern match failure - conn = build_conn() + conn = browser_conn() result = AuthController.callback(conn, %{}) @@ -58,7 +58,7 @@ defmodule WandererAppWeb.AuthControllerTest do test "callback/2 exercises character creation path" do # Test the fallback clause for now since character creation involves complex validation # The actual implementation requires valid EVE character data which is complex to mock - conn = build_conn() + conn = browser_conn() result = AuthController.callback(conn, %{}) @@ -69,7 +69,7 @@ defmodule WandererAppWeb.AuthControllerTest do test "callback/2 handles existing user assignment" do # Test the fallback clause for consistent behavior - conn = build_conn() + conn = browser_conn() result = AuthController.callback(conn, %{}) @@ -81,8 +81,8 @@ defmodule WandererAppWeb.AuthControllerTest do test "callback/2 validates various auth credential formats" do # Test fallback clause behavior for various cases test_cases = [ - build_conn(), - build_conn() |> assign(:some_other_assign, "value") + browser_conn(), + browser_conn() |> assign(:some_other_assign, "value") ] Enum.each(test_cases, fn conn -> @@ -192,4 +192,15 @@ defmodule WandererAppWeb.AuthControllerTest do end) end end + + # The SSO callback failure path calls put_flash/3, which requires the session + # and flash plugs that the real :browser pipeline installs. A bare + # build_conn/0 has neither, so these tests died with "flash not fetched" + # before ever reaching the redirect they assert on. This mirrors the browser + # pipeline rather than relaxing the controller. + defp browser_conn do + build_conn() + |> Plug.Test.init_test_session(%{}) + |> Phoenix.ConnTest.fetch_flash() + end end diff --git a/test/unit/map/map_scopes_test.exs b/test/unit/map/map_scopes_test.exs index 21f07346..421342c2 100644 --- a/test/unit/map/map_scopes_test.exs +++ b/test/unit/map/map_scopes_test.exs @@ -70,6 +70,19 @@ defmodule WandererApp.Map.Server.MapScopesTest do Cachex.put(:system_static_info_cache, solar_system_id, system_info) end) + # :system_static_info_cache is global and shared with every other suite in + # the run. These stub entries carry only :solar_system_id and :system_class, + # so leaving them behind silently replaces the full records other suites + # expect -- 30_000_142 (Jita) in particular is also used by + # CommonAPIControllerTest and OpenAPIValidationTest, which then read a + # record missing solar_system_name and friends. Whether that surfaced + # depended on suite order, which is why these failures moved with --seed. + on_exit(fn -> + Enum.each(Map.keys(test_systems), fn solar_system_id -> + Cachex.del(:system_static_info_cache, solar_system_id) + end) + end) + :ok end