Skip to content

[WIP] Fix GitHub Actions job by increasing test coverage above 95%#3248

Closed
sydseter with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-test-coverage-issue
Closed

[WIP] Fix GitHub Actions job by increasing test coverage above 95%#3248
sydseter with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-test-coverage-issue

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

In OWASP/cornucopia, fix the failing GitHub Actions job for PR #3246 (run 29590186805, job 87917105027) by increasing the COPI application's test coverage above the enforced 95% threshold. The job currently reports total coverage of 93.6% and fails with: FAILED: Expected minimum coverage of 95%, got 93.6%. Review the workflow, coverage configuration, existing tests, and the uncovered code paths reported by ExCoveralls. Add or improve tests only; do not modify production code, coverage thresholds, or CI configuration. Focus on meaningful tests that exercise currently uncovered branches/paths, particularly in the session-store or other files identified by the coverage report. Ensure tests are deterministic, isolated, and compatible with the repository's existing test conventions and database setup. Run the relevant COPI test/coverage commands and verify the resulting total coverage exceeds 95%.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The job fails because the COPI test suite does not meet the configured coverage threshold:

FAILED: Expected minimum coverage of 95%, got 93.6%.
Process completed with exit code 1.

The repeated --- ERROR --- markers around card inserts are not the failing assertion—the queries are reported as QUERY OK. The PostgreSQL role "root" does not exist messages are cleanup/container noise. The failure is coverage enforcement in the reusable workflow called by run-tests-for-patches.yaml.

Recommended fix

Add tests for the uncovered branches in CopiWeb.SessionStore, especially:

  • Cookie-backed get/3, put/3, and delete/3
  • PostgreSQL-backed session retrieval
  • Missing or malformed session cookies
  • Expired sessions
  • Invalid encrypted session data
  • PostgreSQL-backed deletion
  • Session replacement through put/3

The module has two substantially different execution paths controlled by postgres_enabled?/0:

def get(conn, raw_cookie, opts) do
  if postgres_enabled?() do
    get_from_postgres(conn, raw_cookie, opts)
  else
    Plug.Session.COOKIE.get(conn, raw_cookie, opts)
  end
end

A focused test setup can exercise both modes:

setup do
  previous = Application.get_env(:copi, :postgres_session_store_enabled)

  on_exit(fn ->
    if previous == nil do
      Application.delete_env(:copi, :postgres_session_store_enabled)
    else
      Application.put_env(:copi, :postgres_session_store_enabled, previous)
    end
  end)

  :ok
end

defp enable_postgres_sessions do
  Application.put_env(:copi, :postgres_session_store_enabled, true)
end

defp disable_postgres_sessions do
  Application.put_env(:copi, :postgres_session_store_enabled, false)
end

Add cases along these lines:

test "uses the cookie store when PostgreSQL sessions are disabled", %{conn: conn} do
  disable_postgres_sessions()

  opts = SessionStore.init(key: "_session", signing_salt: "test salt")

  {conn, session_id} =
    SessionStore.put(conn, nil, %{"user_id" => 123}, opts)

  assert is_binary(session_id) or session_id == nil
  assert {_id, session} = SessionStore.get(conn, nil, opts)
  assert session["user_id"] == 123
end

test "returns an empty session for an unknown PostgreSQL session", %{conn: conn} do
  enable_postgres_sessions()

  opts = SessionStore.init(key: "_session", signing_salt: "test salt")

  assert {nil, %{}} =
           SessionStore.get(conn, "invalid-cookie-value", opts)
end

test "deletes a PostgreSQL session", %{conn: conn} do
  enable_postgres_sessions()

  opts = SessionStore.init(key: "_session", signing_salt: "test salt")

  {conn, session_id} =
    SessionStore.put(conn, nil, %{"user_id" => 123}, opts)

  assert Repo.get(SessionRecord, session_id)

  SessionStore.delete(conn, session_id, opts)

  refute Repo.get(SessionRecord, session_id)
end

The exact connection/session-cookie handling should follow the existing SessionStore test conventions, but the important point is to cover both configuration branches and the failure paths in get_from_postgres/3.

Avoid weakening the check

Do not solve this by lowering the 95% threshold or marking coverage as allowed to fail. Although mix.exs contains:

test_coverage: [tool: ExCoveralls, allow_failure: true]

the job explicitly fails when coverage is below 95%, as shown in the log. Adding targeted tests for SessionStore should raise the total above the required threshold while validating the newly added session-store behavior.

User: increase test coverage above 95% but do not touch production code

Assistant: We’re waiting for your confirmation to start the coding agent and create a pull request containing test-only changes.

@sydseter sydseter closed this Jul 17, 2026
Copilot AI requested a review from sydseter July 17, 2026 16:05
Copilot stopped work on behalf of sydseter due to an error July 17, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants