From f703e61b1294d36be06239ddf10949509c1c8fb0 Mon Sep 17 00:00:00 2001 From: Graham McIntie Date: Tue, 24 Mar 2026 09:12:59 -0500 Subject: [PATCH] fix: code quality improvements and test reliability (#139) Various code quality improvements and fixes: - Settings: return error instead of auto-creating unknown settings - PagerDuty: proper backoff timing with to_timeout() - Gaiia sync: remove unnecessary transaction wrapper - HTTP/UISP: simplify test plug setup - Agent channel: tagged tuple config for check types - SNMP: fix Task.yield/shutdown pattern, remove stale preload - Discovery: simplify error handling - Tests: bump executor timeouts for CI reliability - Dockerfile: remove unnecessary --force compile - Renovate/CI: config cleanup Split from #135. Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/139 --- .forgejo/workflows/staging.yaml | 1 - .pre-commit-config.yaml | 2 +- assets/js/app.ts | 135 ++++++++---------- k8s/Dockerfile | 4 +- lib/towerops/gaiia/sync.ex | 16 +-- lib/towerops/http.ex | 5 +- lib/towerops/pagerduty/client.ex | 20 +-- lib/towerops/snmp/deferred_discovery.ex | 4 +- lib/towerops/snmp/discovery.ex | 121 +++++++--------- lib/towerops/uisp/client.ex | 5 +- lib/towerops_web/channels/agent_channel.ex | 58 ++++---- mix.lock | 2 +- renovate.json | 1 - .../executors/dns_executor_test.exs | 2 +- .../executors/ping_executor_test.exs | 16 +-- .../executors/ssl_executor_test.exs | 10 +- .../executors/tcp_executor_test.exs | 2 +- test/towerops/settings_test.exs | 13 +- 18 files changed, 184 insertions(+), 233 deletions(-) diff --git a/.forgejo/workflows/staging.yaml b/.forgejo/workflows/staging.yaml index b1cc5671..1e7a0020 100644 --- a/.forgejo/workflows/staging.yaml +++ b/.forgejo/workflows/staging.yaml @@ -15,7 +15,6 @@ concurrency: jobs: deploy-staging: name: Deploy to Dokku Staging - if: ${{ !startsWith(github.head_ref, 'renovate/') }} runs-on: ubuntu-22.04 steps: - name: Checkout code diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 173daa95..3498c83d 120000 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1 +1 @@ -/nix/store/zx636v118rzzqzwafgrw2aybh7l0szrl-pre-commit-config.json \ No newline at end of file +/nix/store/4xlkmdd947h5qlhj2rmbyavq08grkz27-pre-commit-config.json \ No newline at end of file diff --git a/assets/js/app.ts b/assets/js/app.ts index b1620dd0..311049ee 100644 --- a/assets/js/app.ts +++ b/assets/js/app.ts @@ -28,7 +28,6 @@ import Chart from "../vendor/chart" import type { ChartData, ChartDataset, ChartDataPoint } from "../vendor/chart" import cytoscape from "../vendor/cytoscape" import type { SensorChartHook } from "./types/liveview" -import { buildChartDatasets } from "./chart_dataset_config.mjs" import { DeviceListReorder } from "./device_list_reorder" import { initCookieConsent } from "./cookie_consent" @@ -105,7 +104,52 @@ const SensorChart: SensorChartHook = { const range = this.el.dataset.range || '24h' const isTrafficChart = unit === 'bps' - const datasets = buildChartDatasets(data, { isTrafficChart, dualAxis }) as ChartDataset[] + // Generate colors for each dataset + const colors = [ + 'rgb(59, 130, 246)', // blue + 'rgb(239, 68, 68)', // red + 'rgb(34, 197, 94)', // green + 'rgb(234, 179, 8)', // yellow + 'rgb(168, 85, 247)', // purple + 'rgb(236, 72, 153)', // pink + 'rgb(14, 165, 233)', // sky + 'rgb(249, 115, 22)', // orange + ] + + const datasets: ChartDataset[] = data.datasets.map((dataset, index) => { + const color = colors[index % colors.length] + const baseConfig: ChartDataset = { + ...dataset, + borderColor: color, + backgroundColor: color.replace('rgb', 'rgba').replace(')', ', 0.2)'), + borderWidth: 2, + tension: 0.4, + pointRadius: 0, + pointHoverRadius: 4, + } + + // Preserve yAxisID from server data for dual-axis charts + if ((dataset as any).yAxisID) { + (baseConfig as any).yAxisID = (dataset as any).yAxisID + } + + // Status dataset (dual axis, right side) uses stepped line with fill + if (dualAxis && (dataset as any).yAxisID === 'y1') { + (baseConfig as any).stepped = 'before' + baseConfig.tension = 0 + baseConfig.fill = 'origin' + baseConfig.borderWidth = 1 + // Use green with low opacity for the status fill area + baseConfig.borderColor = 'rgb(34, 197, 94)' + baseConfig.backgroundColor = 'rgba(34, 197, 94, 0.1)' + } else if (isTrafficChart) { + baseConfig.fill = 'origin' + } else { + baseConfig.fill = false + } + + return baseConfig + }) // Format bits per second into human-readable units const formatBps = (bps: number): string => { @@ -271,8 +315,7 @@ const SensorChart: SensorChartHook = { return date.toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit', hour12: false }) } }, - maxTicksLimit: 9, - includeBounds: false + maxTicksLimit: 12 }, grid: { display: false @@ -578,10 +621,6 @@ const BetaBannerDismiss = { const NetworkMap = { cy: null as any, _topologyData: null as any, - _zoomInHandler: null as (() => void) | null, - _zoomOutHandler: null as (() => void) | null, - _fitHandler: null as (() => void) | null, - mounted(this: any) { const topologyData = JSON.parse(this.el.dataset.topology || '{}') this._topologyData = topologyData @@ -620,24 +659,6 @@ const NetworkMap = { }, destroyed(this: any) { - // Clean up event listeners - const zoomIn = document.getElementById('cy-zoom-in') - const zoomOut = document.getElementById('cy-zoom-out') - const fit = document.getElementById('cy-fit') - - if (zoomIn && this._zoomInHandler) { - zoomIn.removeEventListener('click', this._zoomInHandler) - this._zoomInHandler = null - } - if (zoomOut && this._zoomOutHandler) { - zoomOut.removeEventListener('click', this._zoomOutHandler) - this._zoomOutHandler = null - } - if (fit && this._fitHandler) { - fit.removeEventListener('click', this._fitHandler) - this._fitHandler = null - } - if (this.cy) { // Stop any running layout before destroying to prevent async callbacks // from accessing a destroyed renderer @@ -657,33 +678,30 @@ const NetworkMap = { const fit = document.getElementById('cy-fit') if (zoomIn) { - this._zoomInHandler = () => { + zoomIn.addEventListener('click', () => { if (!this.cy) return this.cy.zoom({ level: Math.min(this.cy.zoom() + ZOOM_STEP, this.cy.maxZoom()), renderedPosition: { x: this.cy.width() / 2, y: this.cy.height() / 2 } }) - } - zoomIn.addEventListener('click', this._zoomInHandler) + }) } if (zoomOut) { - this._zoomOutHandler = () => { + zoomOut.addEventListener('click', () => { if (!this.cy) return this.cy.zoom({ level: Math.max(this.cy.zoom() - ZOOM_STEP, this.cy.minZoom()), renderedPosition: { x: this.cy.width() / 2, y: this.cy.height() / 2 } }) - } - zoomOut.addEventListener('click', this._zoomOutHandler) + }) } if (fit) { - this._fitHandler = () => { + fit.addEventListener('click', () => { if (!this.cy) return this.cy.fit(undefined, 50) - } - fit.addEventListener('click', this._fitHandler) + }) } }, @@ -1720,21 +1738,11 @@ const userTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone || "UTC" // Global Search (Cmd+K / Ctrl+K) hook const GlobalSearchTrigger = { - handleClick: null as ((e: Event) => void) | null, - - mounted(this: any) { - this.handleClick = () => { + mounted() { + this.el.addEventListener("click", () => { // Simulate Cmd+K to open global search document.dispatchEvent(new KeyboardEvent("keydown", { key: "k", metaKey: true })) - } - this.el.addEventListener("click", this.handleClick) - }, - - destroyed(this: any) { - if (this.handleClick) { - this.el.removeEventListener("click", this.handleClick) - this.handleClick = null - } + }) } } @@ -1774,44 +1782,23 @@ const StatusTitle = { // Sidebar collapse - toggle class on (which LiveView never patches) // so the state persists across all navigations without flicker. const SidebarCollapse = { - handleClick: null as ((e: Event) => void) | null, - mounted(this: any) { - this.handleClick = (e: Event) => { + this.el.addEventListener('click', (e: Event) => { if ((e.target as HTMLElement).closest('[data-sidebar-toggle]')) { const html = document.documentElement html.classList.toggle('sidebar-collapsed') localStorage.setItem('sidebarCollapsed', String(html.classList.contains('sidebar-collapsed'))) } - } - this.el.addEventListener('click', this.handleClick) - }, - - destroyed(this: any) { - if (this.handleClick) { - this.el.removeEventListener('click', this.handleClick) - this.handleClick = null - } + }) } } const ThemeSelector = { - handleThemeChange: null as (() => void) | null, - - mounted(this: any) { + mounted() { this.updateActive() - this.handleThemeChange = () => this.updateActive() - window.addEventListener("phx:set-theme", this.handleThemeChange) + window.addEventListener("phx:set-theme", () => this.updateActive()) }, - - destroyed(this: any) { - if (this.handleThemeChange) { - window.removeEventListener("phx:set-theme", this.handleThemeChange) - this.handleThemeChange = null - } - }, - - updateActive(this: any) { + updateActive() { const current = localStorage.getItem("phx:theme") || "system" this.el.querySelectorAll("[data-phx-theme]").forEach((btn) => { const isActive = btn.dataset.phxTheme === current diff --git a/k8s/Dockerfile b/k8s/Dockerfile index 0aadd874..af5c61d6 100644 --- a/k8s/Dockerfile +++ b/k8s/Dockerfile @@ -80,9 +80,9 @@ RUN cd c_src && make COPY lib lib -# Compile the release (--force prevents stale BEAM files from schema changes) +# Compile the release RUN --mount=type=cache,target=/root/.mix \ - mix compile --force + mix compile COPY assets assets diff --git a/lib/towerops/gaiia/sync.ex b/lib/towerops/gaiia/sync.ex index 7627faab..767e8812 100644 --- a/lib/towerops/gaiia/sync.ex +++ b/lib/towerops/gaiia/sync.ex @@ -11,7 +11,6 @@ defmodule Towerops.Gaiia.Sync do alias Towerops.Gaiia.Client alias Towerops.Gaiia.SubscriberMatching alias Towerops.Integrations - alias Towerops.Repo require Logger @@ -58,24 +57,13 @@ defmodule Towerops.Gaiia.Sync do end defp upsert_nodes(org_id, nodes, type) do - case Repo.transaction(fn -> do_upsert_nodes(org_id, nodes, type) end) do - {:ok, count} -> - count - - {:error, reason} -> - Logger.error("Gaiia #{type} upsert transaction failed: #{inspect(reason)}") - 0 - end - end - - defp do_upsert_nodes(org_id, nodes, type) do Enum.reduce(nodes, 0, fn node, count -> case upsert_node(org_id, node, type) do {:ok, _} -> count + 1 - {:error, cs} -> - Logger.warning("Failed to upsert Gaiia #{type}: #{inspect(cs.errors)}") + {:error, changeset} -> + Logger.warning("Failed to upsert Gaiia #{type}: #{inspect(changeset.errors)}") count end end) diff --git a/lib/towerops/http.ex b/lib/towerops/http.ex index 8f7cc822..a54d06b7 100644 --- a/lib/towerops/http.ex +++ b/lib/towerops/http.ex @@ -36,10 +36,7 @@ defmodule Towerops.HTTP do defp maybe_attach_test_plug(req_opts, owner) do if Application.get_env(:towerops, :env) == :test and !Keyword.has_key?(req_opts, :plug) do - req_opts - |> Keyword.put(:plug, {Req.Test, owner}) - |> Keyword.put_new(:retry, false) - |> Keyword.put_new(:receive_timeout, 1000) + Keyword.put(req_opts, :plug, {Req.Test, owner}) else req_opts end diff --git a/lib/towerops/pagerduty/client.ex b/lib/towerops/pagerduty/client.ex index 1cc5aaa7..f3b7cd75 100644 --- a/lib/towerops/pagerduty/client.ex +++ b/lib/towerops/pagerduty/client.ex @@ -101,7 +101,7 @@ defmodule Towerops.PagerDuty.Client do retry_after = extract_retry_after(response, retries) Logger.info("PagerDuty rate limited, retrying in #{retry_after}ms (attempt #{retries + 1}/#{@max_retries})") - sleep(retry_after) + Process.sleep(retry_after) send_event_with_retry(body, retries + 1) {:ok, %{status: status}} -> @@ -132,22 +132,8 @@ defmodule Towerops.PagerDuty.Client do end end - # Exponential backoff: 1s, 2s, 4s (configurable for tests) - defp backoff_ms(retries) do - # In test env, use 1ms sleep to speed up tests - multiplier = if Application.get_env(:towerops, :env) == :test, do: 1, else: 1000 - round(:math.pow(2, retries)) * multiplier - end - - # Allow mocking sleep in tests - defp sleep(ms) do - # In test env, use minimal sleep - if Application.get_env(:towerops, :env) == :test do - Process.sleep(1) - else - Process.sleep(ms) - end - end + # Exponential backoff: 1s, 2s, 4s + defp backoff_ms(retries), do: to_timeout(second: round(:math.pow(2, retries))) defp dedup_key(alert), do: "towerops-alert-#{alert.id}" diff --git a/lib/towerops/snmp/deferred_discovery.ex b/lib/towerops/snmp/deferred_discovery.ex index 6fd0c328..516e3f95 100644 --- a/lib/towerops/snmp/deferred_discovery.ex +++ b/lib/towerops/snmp/deferred_discovery.ex @@ -60,13 +60,11 @@ defmodule Towerops.Snmp.DeferredDiscovery do task = Task.async(fn -> check_fn.() end) - case Task.yield(task, timeout) do + case Task.yield(task, timeout) || Task.shutdown(task, :brutal_kill) do {:ok, result} -> result nil -> - # Task timed out - kill it and use default - Task.shutdown(task, :brutal_kill) Logger.warning("Slow check timed out after #{timeout}ms, using default value") {:ok, default_value} end diff --git a/lib/towerops/snmp/discovery.ex b/lib/towerops/snmp/discovery.ex index a9c28b7d..206c8cbd 100644 --- a/lib/towerops/snmp/discovery.ex +++ b/lib/towerops/snmp/discovery.ex @@ -233,21 +233,19 @@ defmodule Towerops.Snmp.Discovery do [] end - # Secondary discovery: VLANs, IPs, processors, storage + # Secondary discovery: VLANs, IPs, processors, storage (already fall back to [] on timeout) Logger.info("Discovering VLANs...", device_id: device.id) - vlans = safe_discover(fn -> discover_vlans_with_timeout(client_opts, profile, timeouts) end, "VLAN", device.id) + {:ok, vlans} = discover_vlans_with_timeout(client_opts, profile, timeouts) Logger.info("Discovering IP addresses...", device_id: device.id) - ip_addresses = safe_discover(fn -> discover_ip_addresses_with_timeout(client_opts, timeouts) end, "IP", device.id) + {:ok, ip_addresses} = discover_ip_addresses_with_timeout(client_opts, timeouts) _ = log_ip_discovery_results(device, ip_addresses) Logger.info("Discovering processors...", device_id: device.id) - - processors = - safe_discover(fn -> discover_processors_with_timeout(client_opts, timeouts) end, "Processor", device.id) + {:ok, processors} = discover_processors_with_timeout(client_opts, timeouts) Logger.info("Discovering storage...", device_id: device.id) - storage = safe_discover(fn -> discover_storage_with_timeout(client_opts, timeouts) end, "Storage", device.id) + {:ok, storage} = discover_storage_with_timeout(client_opts, timeouts) Logger.info("Saving discovery results (including IP addresses and processors)...", device_id: device.id) @@ -854,30 +852,25 @@ defmodule Towerops.Snmp.Discovery do Repo.transaction( fn -> # Upsert Device - case upsert_device(device, sanitized_device_info) do - {:ok, snmp_device} -> - # Sync interfaces, sensors, and VLANs (preserving historical data) - synced_interfaces = sync_interfaces(snmp_device, interfaces) - _ = sync_sensors(snmp_device, sensors) - _ = sync_vlans(snmp_device, vlans) + snmp_device = upsert_device(device, sanitized_device_info) - # Sync IP addresses and processors inside transaction for atomicity - device_with_interfaces = %{ - snmp_device - | interfaces: Enum.map(synced_interfaces, &Map.put(&1, :device_id, snmp_device.device_id)) - } + # Sync interfaces, sensors, and VLANs (preserving historical data) + synced_interfaces = sync_interfaces(snmp_device, interfaces) + _ = sync_sensors(snmp_device, sensors) + _ = sync_vlans(snmp_device, vlans) - _ = sync_ip_addresses(device_with_interfaces, ip_addresses) - _ = sync_processors(device_with_interfaces, processors) + # Sync IP addresses and processors inside transaction for atomicity + device_with_interfaces = %{ + snmp_device + | interfaces: Enum.map(synced_interfaces, &Map.put(&1, :device_id, snmp_device.device_id)) + } - # Return device with interfaces loaded and device_id added - # Use snmp_device.device_id which references the Equipment table - device_with_interfaces + _ = sync_ip_addresses(device_with_interfaces, ip_addresses) + _ = sync_processors(device_with_interfaces, processors) - {:error, changeset} -> - Logger.error("Failed to upsert SNMP device: #{inspect(changeset.errors)}") - Repo.rollback({:upsert_failed, changeset}) - end + # Return device with interfaces loaded and device_id added + # Use snmp_device.device_id which references the Equipment table + device_with_interfaces end, timeout: 60_000 ) @@ -919,39 +912,47 @@ defmodule Towerops.Snmp.Discovery do |> Map.put(:last_discovery_at, last_discovery_at) end - @spec upsert_device(DeviceSchema.t(), map()) :: {:ok, Device.t()} | {:error, Ecto.Changeset.t()} + @spec upsert_device(DeviceSchema.t(), map()) :: Device.t() defp upsert_device(device, device_attrs) do Logger.info("Upserting SNMP device for #{device.name}") - existing_device = Repo.get_by(Device, device_id: device.id) - old_version = existing_device && existing_device.firmware_version - new_version = device_attrs[:firmware_version] + case Repo.get_by(Device, device_id: device.id) do + nil -> + snmp_device = + %Device{} + |> Device.changeset(device_attrs) + |> Repo.insert!() - changeset = Device.changeset(existing_device || %Device{}, device_attrs) - - result = - Repo.insert_or_update(changeset, - on_conflict: {:replace_all_except, [:id, :inserted_at]}, - conflict_target: :device_id - ) - - case result do - {:ok, snmp_device} -> - cond do - is_nil(existing_device) && snmp_device.firmware_version -> - Firmware.log_firmware_change(snmp_device.id, nil, snmp_device.firmware_version) - - version_changed?(old_version, new_version) -> - Firmware.log_firmware_change(snmp_device.id, old_version, new_version) - - true -> - :ok + # Log initial firmware version (no old version) + if snmp_device.firmware_version do + Firmware.log_firmware_change( + snmp_device.id, + nil, + snmp_device.firmware_version + ) end - {:ok, snmp_device} + snmp_device - {:error, _changeset} = error -> - error + existing_device -> + old_version = existing_device.firmware_version + new_version = device_attrs[:firmware_version] + + snmp_device = + existing_device + |> Device.changeset(device_attrs) + |> Repo.update!() + + # Detect and log firmware version changes + if version_changed?(old_version, new_version) do + Firmware.log_firmware_change( + snmp_device.id, + old_version, + new_version + ) + end + + snmp_device end end @@ -1508,18 +1509,4 @@ defmodule Towerops.Snmp.Discovery do List.last(group) end) end - - defp safe_discover(discovery_fn, resource_type, device_id) do - case discovery_fn.() do - {:ok, result} -> - result - - {:error, :timeout} -> - [] - - {:error, reason} -> - Logger.warning("#{resource_type} discovery failed: #{inspect(reason)}", device_id: device_id) - [] - end - end end diff --git a/lib/towerops/uisp/client.ex b/lib/towerops/uisp/client.ex index 55715a90..b0d48e6f 100644 --- a/lib/towerops/uisp/client.ex +++ b/lib/towerops/uisp/client.ex @@ -120,10 +120,7 @@ defmodule Towerops.Uisp.Client do # Only use Req.Test plug in test environment opts = if Application.get_env(:towerops, :env) == :test do - opts - |> Keyword.put(:plug, {Req.Test, __MODULE__}) - |> Keyword.put(:retry, false) - |> Keyword.put(:receive_timeout, 1000) + Keyword.put(opts, :plug, {Req.Test, __MODULE__}) else opts end diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index afc396c1..2874a528 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -910,46 +910,54 @@ defmodule ToweropsWeb.AgentChannel do defp check_type_config("http", config) do [ - http: %HttpCheckConfig{ - url: config["url"] || "", - method: config["method"] || "GET", - expected_status: config["expected_status"] || 200, - verify_ssl: config["verify_ssl"] != false, - regex: config["regex"] || "", - follow_redirects: config["follow_redirects"] != false - } + config: + {:http, + %HttpCheckConfig{ + url: config["url"] || "", + method: config["method"] || "GET", + expected_status: config["expected_status"] || 200, + verify_ssl: config["verify_ssl"] != false, + regex: config["regex"] || "", + follow_redirects: config["follow_redirects"] != false + }} ] end defp check_type_config("tcp", config) do [ - tcp: %TcpCheckConfig{ - host: config["host"] || "", - port: config["port"] || 0, - send: config["send"] || "", - expect: config["expect"] || "" - } + config: + {:tcp, + %TcpCheckConfig{ + host: config["host"] || "", + port: config["port"] || 0, + send: config["send"] || "", + expect: config["expect"] || "" + }} ] end defp check_type_config("dns", config) do [ - dns: %DnsCheckConfig{ - hostname: config["hostname"] || "", - server: config["server"] || "", - record_type: config["record_type"] || "A", - expected: config["expected"] || "" - } + config: + {:dns, + %DnsCheckConfig{ + hostname: config["hostname"] || "", + server: config["server"] || "", + record_type: config["record_type"] || "A", + expected: config["expected"] || "" + }} ] end defp check_type_config("ssl", config) do [ - ssl: %SslCheckConfig{ - host: config["host"] || "", - port: config["port"] || 443, - warning_days: config["warning_days"] || 30 - } + config: + {:ssl, + %SslCheckConfig{ + host: config["host"] || "", + port: config["port"] || 443, + warning_days: config["warning_days"] || 30 + }} ] end diff --git a/mix.lock b/mix.lock index 548f4b16..7d962577 100644 --- a/mix.lock +++ b/mix.lock @@ -86,7 +86,7 @@ "stream_data": {:hex, :stream_data, "1.3.0", "bde37905530aff386dea1ddd86ecbf00e6642dc074ceffc10b7d4e41dfd6aac9", [:mix], [], "hexpm", "3cc552e286e817dca43c98044c706eec9318083a1480c52ae2688b08e2936e3c"}, "styler": {:hex, :styler, "1.11.0", "35010d970689a23c2bcc8e97bd8bf7d20e3561d60c49be84654df5c37d051a9c", [:mix], [], "hexpm", "70f36165d0cf238a32b7a456fdef6a9c72e77e657d7ac4a0ace33aeba3f2b8c0"}, "sweet_xml": {:hex, :sweet_xml, "0.7.5", "803a563113981aaac202a1dbd39771562d0ad31004ddbfc9b5090bdcd5605277", [:mix], [], "hexpm", "193b28a9b12891cae351d81a0cead165ffe67df1b73fe5866d10629f4faefb12"}, - "swoosh": {:hex, :swoosh, "1.23.1", "19cbb5d675f272e9df958cbab1e24cb8911038df2b39efda81721b7f7ce7e281", [:mix], [{:bandit, ">= 1.0.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:ex_aws, "~> 2.1", [hex: :ex_aws, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:idna, "~> 6.0", [hex: :idna, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mua, "~> 0.2.3", [hex: :mua, repo: "hexpm", optional: true]}, {:multipart, "~> 0.4", [hex: :multipart, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:req, "~> 0.5.10 or ~> 0.6 or ~> 1.0", [hex: :req, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "3193813b462d6dd519e907c680df04988c47bae372b4159e0c4c9f1c42dffea3"}, + "swoosh": {:hex, :swoosh, "1.24.0", "4df9645aeeef925a2eb10f7a588a6a09ddd6d370c5dfbd3e821b699c574bdf57", [:mix], [{:bandit, ">= 1.0.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:ex_aws, "~> 2.1", [hex: :ex_aws, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:idna, "~> 6.0", [hex: :idna, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mua, "~> 0.2.3", [hex: :mua, repo: "hexpm", optional: true]}, {:multipart, "~> 0.4", [hex: :multipart, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:req, "~> 0.5.10 or ~> 0.6 or ~> 1.0", [hex: :req, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "6ddd84550800468d0e2c15a8aaff924a64c014ed6cff90318077efd1672b8b3b"}, "table_rex": {:hex, :table_rex, "4.1.0", "fbaa8b1ce154c9772012bf445bfb86b587430fb96f3b12022d3f35ee4a68c918", [:mix], [], "hexpm", "95932701df195d43bc2d1c6531178fc8338aa8f38c80f098504d529c43bc2601"}, "tailwind": {:hex, :tailwind, "0.4.1", "e7bcc222fe96a1e55f948e76d13dd84a1a7653fb051d2a167135db3b4b08d3e9", [:mix], [], "hexpm", "6249d4f9819052911120dbdbe9e532e6bd64ea23476056adb7f730aa25c220d1"}, "telemetry": {:hex, :telemetry, "1.4.1", "ab6de178e2b29b58e8256b92b382ea3f590a47152ca3651ea857a6cae05ac423", [:rebar3], [], "hexpm", "2172e05a27531d3d31dd9782841065c50dd5c3c7699d95266b2edd54c2dafa1c"}, diff --git a/renovate.json b/renovate.json index 47ff74ad..efb9d9c0 100644 --- a/renovate.json +++ b/renovate.json @@ -17,7 +17,6 @@ "prConcurrentLimit": 10, "prCreation": "immediate", "automerge": false, - "rebaseWhen": "behind-base-branch", "rangeStrategy": "bump", "separateMajorMinor": true, "separateMinorPatch": false, diff --git a/test/towerops/monitoring/executors/dns_executor_test.exs b/test/towerops/monitoring/executors/dns_executor_test.exs index 8a374740..696a276d 100644 --- a/test/towerops/monitoring/executors/dns_executor_test.exs +++ b/test/towerops/monitoring/executors/dns_executor_test.exs @@ -64,7 +64,7 @@ defmodule Towerops.Monitoring.Executors.DnsExecutorTest do test "returns error for timeout with unreachable server" do # Use a non-routable IP as DNS server to force timeout config = %{"hostname" => "example.com", "server" => "192.0.2.1"} - assert {:error, reason} = DnsExecutor.execute(config, 200) + assert {:error, reason} = DnsExecutor.execute(config, 500) assert is_binary(reason) end end diff --git a/test/towerops/monitoring/executors/ping_executor_test.exs b/test/towerops/monitoring/executors/ping_executor_test.exs index b018d526..4c126e28 100644 --- a/test/towerops/monitoring/executors/ping_executor_test.exs +++ b/test/towerops/monitoring/executors/ping_executor_test.exs @@ -7,7 +7,7 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do test "returns success with valid ping output" do config = %{"host" => "127.0.0.1", "count" => 1} - case PingExecutor.execute(config, 100) do + case PingExecutor.execute(config, 10_000) do {:ok, response_time, output} -> assert is_number(response_time) assert response_time >= 0 @@ -21,10 +21,10 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do end test "defaults count to 3 when not provided" do - config = %{"host" => "127.0.0.1", "count" => 1} + config = %{"host" => "127.0.0.1"} - # Use count=1 to speed up test - result = PingExecutor.execute(config, 100) + # Should use default count of 3 — just verify it doesn't crash + result = PingExecutor.execute(config, 10_000) assert match?({:ok, _, _}, result) or match?({:error, _}, result) end @@ -32,7 +32,7 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do # RFC 5737 TEST-NET address — should be unreachable config = %{"host" => "192.0.2.1", "count" => 1} - case PingExecutor.execute(config, 200) do + case PingExecutor.execute(config, 6000) do {:error, reason} -> assert is_binary(reason) @@ -45,14 +45,14 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do test "returns error for invalid host" do config = %{"host" => "definitely-not-a-real-host-12345.invalid", "count" => 1} - assert {:error, reason} = PingExecutor.execute(config, 200) + assert {:error, reason} = PingExecutor.execute(config, 6000) assert is_binary(reason) end test "sanitizes host to prevent command injection" do config = %{"host" => "127.0.0.1; rm -rf /", "count" => 1} - assert {:error, reason} = PingExecutor.execute(config, 100) + assert {:error, reason} = PingExecutor.execute(config, 5000) assert reason =~ "Invalid host" end @@ -60,7 +60,7 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do config = %{"host" => "127.0.0.1", "count" => -1} # Should clamp or reject negative count - result = PingExecutor.execute(config, 100) + result = PingExecutor.execute(config, 10_000) assert match?({:ok, _, _}, result) or match?({:error, _}, result) end end diff --git a/test/towerops/monitoring/executors/ssl_executor_test.exs b/test/towerops/monitoring/executors/ssl_executor_test.exs index d132b5c8..fa47bd2d 100644 --- a/test/towerops/monitoring/executors/ssl_executor_test.exs +++ b/test/towerops/monitoring/executors/ssl_executor_test.exs @@ -17,7 +17,7 @@ defmodule Towerops.Monitoring.Executors.SslExecutorTest do test "returns error for connection failure to non-routable address" do config = %{"host" => "192.0.2.1", "port" => 443, "warning_days" => 30} - assert {:error, reason} = SslExecutor.execute(config, 300) + assert {:error, reason} = SslExecutor.execute(config, 1_000) assert is_binary(reason) end @@ -29,14 +29,14 @@ defmodule Towerops.Monitoring.Executors.SslExecutorTest do config = %{"host" => "127.0.0.1", "port" => port, "warning_days" => 30} - assert {:error, reason} = SslExecutor.execute(config, 500) + assert {:error, reason} = SslExecutor.execute(config, 2_000) assert is_binary(reason) end test "returns error for missing host key" do config = %{"port" => 443, "warning_days" => 30} - assert {:error, "Exception:" <> _} = SslExecutor.execute(config, 1_000) + assert {:error, "Exception:" <> _} = SslExecutor.execute(config, 5_000) end test "defaults port to 443 when not specified" do @@ -44,13 +44,13 @@ defmodule Towerops.Monitoring.Executors.SslExecutorTest do # but verify the function doesn't crash config = %{"host" => "192.0.2.1", "warning_days" => 30} - assert {:error, _reason} = SslExecutor.execute(config, 200) + assert {:error, _reason} = SslExecutor.execute(config, 500) end test "defaults warning_days to 30 when not specified" do config = %{"host" => "192.0.2.1", "port" => 443} - assert {:error, _reason} = SslExecutor.execute(config, 200) + assert {:error, _reason} = SslExecutor.execute(config, 500) end end diff --git a/test/towerops/monitoring/executors/tcp_executor_test.exs b/test/towerops/monitoring/executors/tcp_executor_test.exs index 2911d7f9..6894593e 100644 --- a/test/towerops/monitoring/executors/tcp_executor_test.exs +++ b/test/towerops/monitoring/executors/tcp_executor_test.exs @@ -66,7 +66,7 @@ defmodule Towerops.Monitoring.Executors.TcpExecutorTest do test "returns error for connection timeout" do # Use a non-routable IP to force timeout config = %{"host" => "192.0.2.1", "port" => 80} - assert {:error, reason} = TcpExecutor.execute(config, 200) + assert {:error, reason} = TcpExecutor.execute(config, 500) assert String.contains?(reason, "timeout") or String.contains?(reason, "unreachable") or String.contains?(reason, "failed") diff --git a/test/towerops/settings_test.exs b/test/towerops/settings_test.exs index ca8c6898..d18ddb11 100644 --- a/test/towerops/settings_test.exs +++ b/test/towerops/settings_test.exs @@ -4,9 +4,17 @@ defmodule Towerops.SettingsTest do alias Towerops.Settings alias Towerops.Settings.ApplicationSetting + # Ensure the migration-seeded setting exists (CI may have stale test DB) + setup do + unless Settings.get_setting_record("global_default_cloud_poller_id") do + {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") + end + + :ok + end + describe "get_setting/1" do test "returns parsed value for existing setting" do - {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") assert Settings.get_setting("global_default_cloud_poller_id") == nil end @@ -75,7 +83,6 @@ defmodule Towerops.SettingsTest do describe "get_setting_record/1" do test "returns the raw ApplicationSetting record" do - {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") record = Settings.get_setting_record("global_default_cloud_poller_id") assert %ApplicationSetting{} = record assert record.key == "global_default_cloud_poller_id" @@ -89,7 +96,6 @@ defmodule Towerops.SettingsTest do describe "update_setting/2" do test "updates an existing setting value" do - {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") agent_token_id = Ecto.UUID.generate() {:ok, updated} = Settings.update_setting("global_default_cloud_poller_id", agent_token_id) @@ -98,7 +104,6 @@ defmodule Towerops.SettingsTest do end test "updates setting to nil" do - {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") agent_token_id = Ecto.UUID.generate() {:ok, _} = Settings.update_setting("global_default_cloud_poller_id", agent_token_id) {:ok, updated} = Settings.update_setting("global_default_cloud_poller_id", nil)