diff --git a/README.md b/README.md index afd496c..5ac7658 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,11 @@ crawlscope validate --url https://example.com --sitemap https://example.com/site Child sitemap indexes are supported automatically. +Set `CRAWLSCOPE_PROFILE_TOKEN` through the process environment or your secret +manager when the site protects diagnostic timing with `X-Profile-Token`. +Crawlscope intentionally has no `--profile-token` flag because command arguments +can be exposed through shell history and process listings. + Validation output is grouped for terminal scanning: ```text @@ -141,10 +146,69 @@ puts result.issues.to_a.map(&:message) - `urls`: sitemap URLs selected for validation - `pages`: fetched page snapshots - `issues`: structured issues with `code`, `severity`, `category`, `url`, and `message` +- `server_timing_summary`: aggregate response timing data when pages publish it `result.ok?` returns `false` when an error is present. Warnings and notices remain available through `result.issues` without making the result fail. +## Server Timing + +Crawlscope parses the +[`Server-Timing`](https://www.w3.org/TR/server-timing/) response header for both +HTTP and browser-rendered crawls. Each page exposes its parsed header through +`page.server_timing`: + +```ruby +page.server_timing.each do |metric| + puts [metric.name, metric.duration, metric.description].compact.join(": ") +end +``` + +The validation report adds a `Server Timing` section only when at least one page +publishes the header. It includes: + +- header coverage and the number of pages publishing durations +- sample and page counts with average, p50, p95, and maximum per metric +- non-duration signals, including cache status and routing descriptions +- the ten pages with the largest individual metric +- the number of malformed entries ignored during parsing + +`dur` values are reported as milliseconds, as recommended by the specification. +Crawlscope does not add durations together because metrics such as `total`, +`app`, and `db` may overlap. A page's worst-offender entry is its largest +individual metric instead. + +The current HTTP and browser transports expose response headers, not response +trailers. Metrics published only in a `Server-Timing` trailer are therefore not +available to Crawlscope. + +Rails 8 applications can enable `ActionDispatch::ServerTiming` with: + +```ruby +config.server_timing = true +``` + +Applications that expose timing only to authenticated diagnostics can set +`config.profile_token`. When this value is present, Crawlscope sends it as +`X-Profile-Token` on same-origin sitemap, HTTP, and browser document requests. +The header survives same-origin redirects but is removed before a cross-origin +redirect, and browser subresources never receive it. Keep the token in the host +application's secret store rather than a URL, command argument, report, or +checked-in configuration. + +`CRAWLSCOPE_PROFILE_TOKEN` is the portable default for Rails, the standalone +CLI, Rake tasks, and plain Ruby callers. Rails applications may instead assign +`config.profile_token` from encrypted credentials when that is their established +secret-management boundary; explicit configuration takes precedence over the +environment. + +New Rails applications enable it in development by default; production remains +opt-in. Rails publishes the Active Support notification names observed during +each request and sums repeated events with the same name. The exact metrics +therefore depend on the application and request, but commonly include controller, +view, database, and cache instrumentation. Crawlscope accepts Rails' dotted +metric names and reports each metric independently. + ## Rails Usage Run the install generator after adding the gem: @@ -162,10 +226,11 @@ Customize the `Crawlscope.configure` block inside the generated initializer: ```ruby Crawlscope.configure do |config| - config.base_url = -> { "https://example.com" } - config.sitemap_path = -> { Rails.public_path.join("sitemap.xml").to_s } - config.site_name = "Example" - config.schema_registry = -> { Crawlscope::SchemaRegistry.default } + config.base_url = -> { ENV.fetch("CRAWLSCOPE_BASE_URL", "http://localhost:3000") } + config.sitemap_path = lambda { + ENV.fetch("SITEMAP", "#{config.base_url.to_s.chomp("/")}/sitemap.xml") + } + config.site_name = ENV.fetch("CRAWLSCOPE_SITE_NAME", "Application") end ``` @@ -193,6 +258,7 @@ Available environment overrides: - `URL` - `SITEMAP` +- `CRAWLSCOPE_PROFILE_TOKEN` - `RULES=metadata,links` - `JS=1` or `RENDERER=browser` - `TIMEOUT=30` @@ -226,10 +292,10 @@ bundle exec rake 'crawlscope:validate:ldjson[https://example.com/article]' `crawlscope:validate` runs all default sitemap rules: indexability, metadata, structured data, uniqueness, content quality, and links. `URL` is the site -base. Without `SITEMAP`, Crawlscope uses the configured sitemap path, then -falls back to `/sitemap.xml`. With `SITEMAP`, Crawlscope uses `URL` as the site -base and validates URLs from that sitemap. `SITEMAP` may be a full URL or a -local file path. +base. Without `SITEMAP`, Crawlscope uses the configured sitemap URL, then fetches +`/sitemap.xml` from `URL` over HTTP. With `SITEMAP`, Crawlscope uses `URL` as +the site base and validates URLs from that sitemap. `SITEMAP` may be a full URL +or an explicitly selected local file path. Plain `rake` does not pass `--url` style flags to tasks. Use `URL=...` or the task-argument form above instead. diff --git a/UPGRADE.md b/UPGRADE.md index f3c9415..e718b28 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -6,6 +6,34 @@ behavior. ## Next Release +### Default sitemap is fetched over HTTP + +Crawlscope no longer prefers `public/sitemap.xml` when validating a localhost +application. Without an explicit `SITEMAP` or configured `sitemap_path`, it now +fetches `/sitemap.xml` from the configured base URL so the crawl observes the +live application and database state. + +The Rails installer now generates the same HTTP default. Regenerate or update +existing initializers that use `Rails.public_path.join("sitemap.xml")`. +Explicit local sitemap paths remain supported through `SITEMAP`, `--sitemap`, +or `config.sitemap_path`. + +`CRAWLSCOPE_PROFILE_TOKEN` is now the portable default for standalone CLI, Rake, +Rails, and plain Ruby usage. Explicit `config.profile_token` values still take +precedence. + +### Server-Timing report output + +No host configuration is required. When one or more responses publish a +`Server-Timing` header, the text report now includes an optional `Server Timing` +section. Host applications that parse report text should accept this additional +section. + +Parsed metrics are available through `page.server_timing`, and aggregate data is +available through `result.server_timing_summary`. Crawlscope interprets `dur` +values as milliseconds and ignores malformed entries while reporting their +count. + ### Ruby 3.3 is now required Crawlscope now depends on the current Async runtime for production async HTTP diff --git a/lib/crawlscope/browser.rb b/lib/crawlscope/browser.rb index 2e7db5a..7216570 100644 --- a/lib/crawlscope/browser.rb +++ b/lib/crawlscope/browser.rb @@ -4,13 +4,15 @@ module Crawlscope class Browser - def initialize(base_url:, timeout_seconds:, network_idle_timeout_seconds:, scroll_page:) + def initialize(base_url:, timeout_seconds:, network_idle_timeout_seconds:, scroll_page:, profile_token: nil) @base_url = base_url @timeout_seconds = timeout_seconds @network_idle_timeout_seconds = network_idle_timeout_seconds + @profile_token = profile_token @scroll_page = scroll_page @browser = build_browser @page = @browser.create_page + configure_profile_requests end def close @@ -72,6 +74,21 @@ def build_browser ) end + def configure_profile_requests + return if @profile_token.to_s.empty? + + @page.network.intercept(resource_type: :document) + @page.on(:request) do |request| + headers = RequestHeaders.add_profile_token( + request.headers.dup, + url: request.url, + base_url: @base_url, + profile_token: @profile_token + ) + request.continue(headers: headers.map { |name, value| {name: name.to_s, value: value.to_s} }) + end + end + def scroll_for_render @page.evaluate("(function() { if (document.body) { window.scrollTo(0, document.body.scrollHeight); } })()") wait_for_network_idle diff --git a/lib/crawlscope/configuration.rb b/lib/crawlscope/configuration.rb index 76df814..1ada579 100644 --- a/lib/crawlscope/configuration.rb +++ b/lib/crawlscope/configuration.rb @@ -11,7 +11,7 @@ class Configuration RENDERERS = %i[http browser].freeze DEFAULT_TIMEOUT_SECONDS = 20 - attr_writer :allowed_statuses, :base_url, :browser_factory, :concurrency, :fetch_executor, :network_idle_timeout_seconds, :output, :renderer, :rule_registry, :schema_registry, :scroll_page, :site_name, :sitemap_path, :timeout_seconds + attr_writer :allowed_statuses, :base_url, :browser_factory, :concurrency, :fetch_executor, :network_idle_timeout_seconds, :output, :profile_token, :renderer, :rule_registry, :schema_registry, :scroll_page, :site_name, :sitemap_path, :timeout_seconds def allowed_statuses value = resolve(@allowed_statuses) @@ -59,6 +59,10 @@ def output value.nil? ? $stdout : value end + def profile_token + resolve(@profile_token) || ENV["CRAWLSCOPE_PROFILE_TOKEN"] + end + def renderer value = resolve(@renderer) normalized_value = value.to_s.strip @@ -93,6 +97,7 @@ def audit(base_url: self.base_url, sitemap_path: self.sitemap_path, rule_names: concurrency: concurrency, fetch_executor: fetch_executor, network_idle_timeout_seconds: network_idle_timeout_seconds, + profile_token: profile_token, renderer: renderer, timeout_seconds: timeout_seconds, allowed_statuses: allowed_statuses, diff --git a/lib/crawlscope/crawl.rb b/lib/crawlscope/crawl.rb index 4bb42dc..bf2aea1 100644 --- a/lib/crawlscope/crawl.rb +++ b/lib/crawlscope/crawl.rb @@ -2,7 +2,7 @@ module Crawlscope class Crawl - def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_factory: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: nil, network_idle_timeout_seconds: Configuration::DEFAULT_BROWSER_NETWORK_IDLE_TIMEOUT_SECONDS, renderer: :http, scroll_page: Configuration::DEFAULT_BROWSER_SCROLL_PAGE, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS, allowed_statuses: Configuration::DEFAULT_ALLOWED_STATUSES) + def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_factory: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: nil, network_idle_timeout_seconds: Configuration::DEFAULT_BROWSER_NETWORK_IDLE_TIMEOUT_SECONDS, profile_token: nil, renderer: :http, scroll_page: Configuration::DEFAULT_BROWSER_SCROLL_PAGE, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS, allowed_statuses: Configuration::DEFAULT_ALLOWED_STATUSES) @base_url = base_url @sitemap_path = sitemap_path @rules = Array(rules) @@ -10,6 +10,7 @@ def initialize(base_url:, sitemap_path:, rules:, schema_registry:, browser_facto @browser_factory = browser_factory @concurrency = concurrency @network_idle_timeout_seconds = network_idle_timeout_seconds + @profile_token = profile_token @renderer = renderer.to_sym @fetch_executor = fetch_executor || default_fetch_executor @scroll_page = scroll_page @@ -49,6 +50,7 @@ def sitemap_urls adapter: http_adapter, concurrency: @concurrency, fetch_executor: @fetch_executor, + profile_token: @profile_token, timeout_seconds: @timeout_seconds ).urls(base_url: @base_url) raise ValidationError, "No URLs found in sitemap: #{@sitemap_path}" if urls.empty? @@ -61,6 +63,7 @@ def browser base_url: @base_url, timeout_seconds: @timeout_seconds, network_idle_timeout_seconds: @network_idle_timeout_seconds, + profile_token: @profile_token, scroll_page: @scroll_page ) rescue LoadError => error @@ -71,7 +74,7 @@ def page if @renderer == :browser (@browser_factory || method(:browser)).call else - Http.new(base_url: @base_url, timeout_seconds: @timeout_seconds, adapter: http_adapter) + Http.new(base_url: @base_url, timeout_seconds: @timeout_seconds, adapter: http_adapter, profile_token: @profile_token) end end diff --git a/lib/crawlscope/http.rb b/lib/crawlscope/http.rb index d9eaed7..745736d 100644 --- a/lib/crawlscope/http.rb +++ b/lib/crawlscope/http.rb @@ -10,10 +10,11 @@ class Http MAX_REDIRECTS = 5 USER_AGENT = "Mozilla/5.0 (compatible; Crawlscope/1.0)" - def initialize(base_url:, timeout_seconds:, adapter: nil) + def initialize(base_url:, timeout_seconds:, adapter: nil, profile_token: nil) @base_url = base_url @timeout_seconds = timeout_seconds @adapter = adapter + @profile_token = profile_token @connections_by_thread = Concurrent::Map.new end @@ -28,6 +29,12 @@ def close def fetch(url) response = connection.get(url) do |request| request.headers["User-Agent"] = USER_AGENT + RequestHeaders.add_profile_token( + request.headers, + url: url, + base_url: @base_url, + profile_token: @profile_token + ) end final_url = response.env.url.to_s @@ -67,7 +74,11 @@ def fetch(url) def connection @connections_by_thread.compute_if_absent(Thread.current.object_id) do Faraday.new do |faraday| - faraday.response :follow_redirects, limit: MAX_REDIRECTS + faraday.response( + :follow_redirects, + limit: MAX_REDIRECTS, + callback: RequestHeaders.method(:strip_profile_token_on_cross_origin_redirect) + ) faraday.options.timeout = @timeout_seconds faraday.options.open_timeout = @timeout_seconds faraday.adapter @adapter if @adapter diff --git a/lib/crawlscope/page.rb b/lib/crawlscope/page.rb index 1de0037..65ee75f 100644 --- a/lib/crawlscope/page.rb +++ b/lib/crawlscope/page.rb @@ -19,5 +19,13 @@ def initialize(url:, normalized_url:, final_url:, normalized_final_url:, status: def html? !doc.nil? end + + def header(name) + headers.find { |key, _value| key.to_s.casecmp?(name.to_s) }&.last + end + + def server_timing + @server_timing ||= ServerTiming.new(header("server-timing")) + end end end diff --git a/lib/crawlscope/reporter.rb b/lib/crawlscope/reporter.rb index 4c8e1c0..370fbf3 100644 --- a/lib/crawlscope/reporter.rb +++ b/lib/crawlscope/reporter.rb @@ -19,13 +19,18 @@ def report(result) if result.issues.size.zero? @io.puts("Status: OK") - return + else + @io.puts("Status: #{status_for(result.issues)}") + @io.puts("Issues: #{result.issues.size} total (#{severity_summary(result.issues)})") end - @io.puts("Status: #{status_for(result.issues)}") - @io.puts("Issues: #{result.issues.size} total (#{severity_summary(result.issues)})") - @io.puts("") + ServerTiming::Reporter.new(io: @io).report( + result.server_timing_summary, + base_url: result.base_url + ) + return if result.issues.size.zero? + @io.puts("") report_summary(result.issues) @io.puts("") report_issue_groups(result.issues, base_url: result.base_url) diff --git a/lib/crawlscope/request_headers.rb b/lib/crawlscope/request_headers.rb new file mode 100644 index 0000000..384b5c0 --- /dev/null +++ b/lib/crawlscope/request_headers.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "uri" + +module Crawlscope + module RequestHeaders + PROFILE_TOKEN = "X-Profile-Token" + + module_function + + def add_profile_token(headers, url:, base_url:, profile_token:) + return headers if profile_token.to_s.empty? + return headers unless same_origin?(url, base_url: base_url) + + headers[PROFILE_TOKEN] = profile_token + headers + end + + def strip_profile_token_on_cross_origin_redirect(response_env, request_env) + return if same_origin?(request_env[:url], base_url: response_env[:url]) + + request_env[:request_headers].delete(PROFILE_TOKEN) + end + + def same_origin?(url, base_url:) + url_uri = URI.join(base_url.to_s, url.to_s) + base_uri = URI.parse(base_url.to_s) + + [url_uri.scheme, url_uri.host, url_uri.port] == [base_uri.scheme, base_uri.host, base_uri.port] + rescue URI::Error + false + end + end +end diff --git a/lib/crawlscope/result.rb b/lib/crawlscope/result.rb index bf3f522..f2b09ba 100644 --- a/lib/crawlscope/result.rb +++ b/lib/crawlscope/result.rb @@ -5,5 +5,9 @@ module Crawlscope def ok? issues.none?(&:error?) end + + def server_timing_summary + ServerTiming::Summary.new(pages) + end end end diff --git a/lib/crawlscope/run.rb b/lib/crawlscope/run.rb index 6944c12..ed63522 100644 --- a/lib/crawlscope/run.rb +++ b/lib/crawlscope/run.rb @@ -44,17 +44,7 @@ def default_sitemap_path(base_url:) value = @configuration.sitemap_path return value unless value.to_s.strip.empty? - local_path = File.expand_path("public/sitemap.xml", Dir.pwd) - return local_path if local_path_default?(base_url: base_url) && File.exist?(local_path) - "#{base_url.to_s.chomp("/")}/sitemap.xml" end - - def local_path_default?(base_url:) - host = URI.parse(base_url.to_s).host.to_s - ["localhost", "127.0.0.1"].include?(host) - rescue URI::InvalidURIError - false - end end end diff --git a/lib/crawlscope/server_timing.rb b/lib/crawlscope/server_timing.rb new file mode 100644 index 0000000..96c26cb --- /dev/null +++ b/lib/crawlscope/server_timing.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +module Crawlscope + class ServerTiming + include Enumerable + + TOKEN = /\A[!#$%&'*+\-.^_`|~0-9A-Za-z]+\z/ + NUMBER = /\A-?(?:(?:\d+(?:\.\d*)?)|(?:\.\d+))(?:[eE][+-]?\d+)?\z/ + QUOTED_STRING = /\A"(?:[^"\\\x00-\x1F]|\\[\t\x20-\x7E])*"\z/ + EMPTY_LIST_MEMBER = /\A[ \t]*\z/ + + Metric = Data.define(:name, :duration, :description) do + def timed? + !duration.nil? + end + end + + attr_reader :invalid_count + + def initialize(header) + @header = header + @invalid_count = 0 + @metrics = parse.freeze + end + + def each(&block) + @metrics.each(&block) + end + + def empty? + @metrics.empty? + end + + def present? + !@header.nil? + end + + def size + @metrics.size + end + + private + + def parse + Array(@header).flatten.compact.flat_map do |header| + metrics_from(header.to_s) + end + end + + def metrics_from(header) + fields, complete = split(header, ",") + + unless complete + @invalid_count += 1 + fields.pop + end + + fields.filter_map do |field| + metric_from(field) unless EMPTY_LIST_MEMBER.match?(field) + end + end + + def metric_from(field) + parts, complete = split(field, ";") + name = parts.shift.to_s.strip + + if complete && TOKEN.match?(name) + metric(name, parameters_from(parts)) + else + @invalid_count += 1 + nil + end + end + + def parameters_from(parts) + parts.each_with_object({}) do |part, parameters| + name, value = part.strip.split("=", 2) + next unless name + + name = name.downcase + next if parameters.key?(name) + + parameters[name] = parameter_value(value.to_s.strip) + end + end + + def parameter_value(value) + if TOKEN.match?(value) + value + elsif QUOTED_STRING.match?(value) + value[1...-1].gsub(/\\(.)/m, '\1') + end + end + + def metric(name, parameters) + duration = duration_from(parameters["dur"]) + + if (parameters.key?("dur") && duration.nil?) || + (parameters.key?("desc") && parameters["desc"].nil?) + @invalid_count += 1 + nil + else + Metric.new( + name: name, + duration: duration, + description: parameters["desc"] + ) + end + end + + def duration_from(value) + if value && NUMBER.match?(value) + duration = Float(value) + duration if duration.finite? + end + end + + def split(value, delimiter) + fields, quoted, escaped = [+""], false, false + + value.each_char do |character| + if escaped + escaped = false + elsif quoted && character == "\\" + escaped = true + elsif character == "\"" + quoted = !quoted + elsif character == delimiter && !quoted + fields << +"" + end + + fields.last << character unless character == delimiter && !quoted + end + + [fields, !quoted && !escaped] + end + end +end diff --git a/lib/crawlscope/server_timing/reporter.rb b/lib/crawlscope/server_timing/reporter.rb new file mode 100644 index 0000000..405d0dc --- /dev/null +++ b/lib/crawlscope/server_timing/reporter.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require "uri" + +module Crawlscope + class ServerTiming + class Reporter + def initialize(io:) + @io = io + end + + def report(summary, base_url:) + return unless summary.present? + + @io.puts("") + @io.puts("Server Timing:") + @io.puts( + " Coverage: #{summary.coverage}/#{summary.pages.size} pages " \ + "(#{percentage(summary.coverage, summary.pages.size)}); " \ + "durations on #{summary.duration_pages} #{pluralize(:page, summary.duration_pages)}" + ) + + report_metrics(summary.metrics) + report_signals(summary.signals) + report_worst_pages(summary.worst_pages, base_url: base_url) + @io.puts(" Ignored malformed entries: #{summary.invalid_count}") if summary.invalid_count.positive? + end + + private + + def report_metrics(metrics) + return if metrics.empty? + + @io.puts(" Duration metrics (dur; milliseconds assumed):") + metrics.each do |metric| + label = metric[:name] + label += " #{description(metric[:description])}" if metric[:description] + + @io.puts( + " #{label}: " \ + "#{metric[:samples]} #{pluralize(:sample, metric[:samples])} / " \ + "#{metric[:pages]} #{pluralize(:page, metric[:pages])}; " \ + "avg #{duration(metric[:average])}; " \ + "p50 #{duration(metric[:p50])}; " \ + "p95 #{duration(metric[:p95])}; " \ + "max #{duration(metric[:maximum])}" + ) + end + end + + def report_signals(signals) + return if signals.empty? + + @io.puts(" Signals:") + signals.each do |signal| + label = signal[:name] + label += " #{description(signal[:description])}" if signal[:description] + + @io.puts( + " #{label}: " \ + "#{signal[:samples]} #{pluralize(:sample, signal[:samples])} / " \ + "#{signal[:pages]} #{pluralize(:page, signal[:pages])}" + ) + end + end + + def report_worst_pages(samples, base_url:) + return if samples.empty? + + @io.puts(" Worst pages:") + samples.each do |sample| + metric = sample[:metric] + detail = metric.description ? " #{description(metric.description)}" : "" + @io.puts( + " #{relative_url(sample[:page].url, base_url: base_url)}: " \ + "#{duration(metric.duration)} (#{metric.name}#{detail})" + ) + end + end + + def duration(value) + "#{format("%.3f", value).sub(/\.?0+\z/, "")}ms" + end + + def description(value) + value = value.to_s + value = "#{value[0, 77]}..." if value.length > 80 + value.inspect + end + + def percentage(numerator, denominator) + return "0.0%" if denominator.zero? + + format("%.1f%%", numerator.fdiv(denominator) * 100) + end + + def relative_url(url, base_url:) + uri = URI.parse(url) + base_uri = URI.parse(base_url) + return url unless uri.host == base_uri.host && uri.scheme == base_uri.scheme && uri.port == base_uri.port + + uri.request_uri + rescue URI::InvalidURIError + url + end + + def pluralize(word, count) + (count == 1) ? word.to_s : "#{word}s" + end + end + end +end diff --git a/lib/crawlscope/server_timing/summary.rb b/lib/crawlscope/server_timing/summary.rb new file mode 100644 index 0000000..b5092ac --- /dev/null +++ b/lib/crawlscope/server_timing/summary.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module Crawlscope + class ServerTiming + class Summary + WORST_PAGE_LIMIT = 10 + + attr_reader :pages + + def initialize(pages) + @pages = pages + @timings = pages.filter_map do |page| + timing = page.server_timing + [page, timing] if timing.present? + end + end + + def present? + @timings.any? + end + + def duration_pages + @timings.count { |_page, timing| timing.any?(&:timed?) } + end + + def invalid_count + @timings.sum { |_page, timing| timing.invalid_count } + end + + def metrics + timed_metrics + .group_by { |_page, metric| [metric.name, metric.description] } + .map { |(name, description), samples| metric_summary(name, description, samples) } + .sort_by { |metric| [-metric[:p95], -metric[:maximum], metric[:name]] } + end + + def signals + all_metrics + .reject { |_page, metric| metric.timed? } + .group_by { |_page, metric| [metric.name, metric.description] } + .map { |(name, description), samples| signal_summary(name, description, samples) } + .sort_by { |signal| [-signal[:samples], signal[:name], signal[:description].to_s] } + end + + def worst_pages(limit: WORST_PAGE_LIMIT) + @timings + .filter_map { |page, timing| worst_metric(page, timing) } + .min_by(limit) { |sample| [-sample[:metric].duration, sample[:page].url] } + .sort_by { |sample| [-sample[:metric].duration, sample[:page].url] } + end + + def coverage + @timings.size + end + + private + + def timed_metrics + all_metrics.select { |_page, metric| metric.timed? } + end + + def all_metrics + @all_metrics ||= @timings.flat_map do |page, timing| + timing.map { |metric| [page, metric] } + end + end + + def metric_summary(name, description, samples) + durations = samples.map { |_page, metric| metric.duration }.sort + + { + name: name, + description: description, + samples: samples.size, + pages: samples.map(&:first).uniq.size, + average: durations.sum.fdiv(durations.size), + p50: percentile(durations, 0.50), + p95: percentile(durations, 0.95), + maximum: durations.last + } + end + + def signal_summary(name, description, samples) + { + name: name, + description: description, + samples: samples.size, + pages: samples.map(&:first).uniq.size + } + end + + def percentile(values, percentile) + position = (values.size - 1) * percentile + lower = values[position.floor] + upper = values[position.ceil] + + lower + (upper - lower) * (position - position.floor) + end + + def worst_metric(page, timing) + if (metric = timing.select(&:timed?).max_by(&:duration)) + {page: page, metric: metric} + end + end + end + end +end diff --git a/lib/crawlscope/sitemap.rb b/lib/crawlscope/sitemap.rb index 8ebfb33..7eb7f69 100644 --- a/lib/crawlscope/sitemap.rb +++ b/lib/crawlscope/sitemap.rb @@ -9,11 +9,12 @@ module Crawlscope class Sitemap SITEMAP_NAMESPACE = {"xmlns" => "http://www.sitemaps.org/schemas/sitemap/0.9"}.freeze - def initialize(path:, adapter: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: Configuration::DEFAULT_FETCH_EXECUTOR, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS) + def initialize(path:, adapter: nil, concurrency: Configuration::DEFAULT_CONCURRENCY, fetch_executor: Configuration::DEFAULT_FETCH_EXECUTOR, profile_token: nil, timeout_seconds: Configuration::DEFAULT_TIMEOUT_SECONDS) @path = path @adapter = adapter @concurrency = concurrency @fetch_executor = fetch_executor + @profile_token = profile_token @timeout_seconds = timeout_seconds end @@ -34,7 +35,7 @@ def collect_urls(source, base_url:, visited:, visited_mutex:) end return [] if already_visited - document = Nokogiri::XML(read(source)) + document = Nokogiri::XML(read(source, base_url: base_url)) root_name = document.root&.name unless %w[sitemapindex urlset].include?(root_name) raise ValidationError, "Sitemap #{source} has unexpected root #{root_name.inspect}" @@ -55,9 +56,16 @@ def collect_urls(source, base_url:, visited:, visited_mutex:) end end - def read(source) + def read(source, base_url:) if Url.remote?(source) - response = connection.get(source) + response = connection.get(source) do |request| + RequestHeaders.add_profile_token( + request.headers, + url: source, + base_url: base_url, + profile_token: @profile_token + ) + end unless response.status.to_i.between?(200, 299) raise ValidationError, "Sitemap #{source} returned HTTP #{response.status}" end @@ -92,7 +100,11 @@ def local_child_path(parent_source, child_loc) def connection Faraday.new do |faraday| - faraday.response :follow_redirects, limit: Http::MAX_REDIRECTS + faraday.response( + :follow_redirects, + limit: Http::MAX_REDIRECTS, + callback: RequestHeaders.method(:strip_profile_token_on_cross_origin_redirect) + ) faraday.options.timeout = @timeout_seconds faraday.options.open_timeout = @timeout_seconds faraday.adapter @adapter if @adapter diff --git a/lib/generators/crawlscope/templates/initializer.rb.tt b/lib/generators/crawlscope/templates/initializer.rb.tt index 1a18826..cd804ea 100644 --- a/lib/generators/crawlscope/templates/initializer.rb.tt +++ b/lib/generators/crawlscope/templates/initializer.rb.tt @@ -14,7 +14,7 @@ module CrawlscopeConfiguration Crawlscope.configure do |config| config.base_url = -> { ENV.fetch("CRAWLSCOPE_BASE_URL", "http://localhost:3000") } config.sitemap_path = lambda { - ENV.fetch("SITEMAP", Rails.public_path.join("sitemap.xml").to_s) + ENV.fetch("SITEMAP", "#{config.base_url.to_s.chomp("/")}/sitemap.xml") } config.site_name = ENV.fetch("CRAWLSCOPE_SITE_NAME", "Application") end diff --git a/test/crawlscope/browser_test.rb b/test/crawlscope/browser_test.rb index 4d6058a..48e1556 100644 --- a/test/crawlscope/browser_test.rb +++ b/test/crawlscope/browser_test.rb @@ -14,13 +14,14 @@ def quit end class FakeNetwork - attr_reader :cleared, :idle_waits, :status + attr_reader :cleared, :idle_waits, :interceptions, :status def initialize(response:, status: 200) @response = response @status = status @cleared = [] @idle_waits = [] + @interceptions = [] end def clear(scope) @@ -29,6 +30,10 @@ def clear(scope) attr_reader :response + def intercept(**options) + @interceptions << options + end + def wait_for_idle(duration:, timeout:) @idle_waits << {duration: duration, timeout: timeout} end @@ -43,6 +48,7 @@ def initialize(network:, body: "", current_url: "", url: "") @current_url = current_url @url = url @evaluations = [] + @callbacks = {} end attr_reader :body @@ -57,11 +63,40 @@ def go_to(url) @visited_url = url end + def on(event, &callback) + @callbacks[event] = callback + end + + def publish(event, value) + @callbacks.fetch(event).call(value) + end + attr_reader :url end + class FakeRequest + attr_reader :continued_headers, :headers, :url + + def initialize(url:, headers: {}) + @url = url + @headers = headers + end + + def continue(headers:) + @continued_headers = headers.to_h { |header| [header.fetch(:name), header.fetch(:value)] } + end + end + def test_fetch_returns_rendered_page - network = FakeNetwork.new(response: Response.new(url: "https://example.com/final", headers: {"content-type" => "text/html"})) + network = FakeNetwork.new( + response: Response.new( + url: "https://example.com/final", + headers: { + "content-type" => "text/html", + "server-timing" => "app;dur=47.2" + } + ) + ) page = FakePage.new(network: network, body: "Hello") browser = browser_with(page: page, scroll_page: false) @@ -73,6 +108,7 @@ def test_fetch_returns_rendered_page assert_equal "https://example.com/final", result.normalized_final_url assert_equal 200, result.status assert result.html? + assert_equal 47.2, result.server_timing.first.duration assert_equal [], page.evaluations end @@ -88,6 +124,21 @@ def test_fetch_scrolls_when_enabled assert_equal 4, network.idle_waits.size end + def test_profile_token_is_limited_to_same_origin_documents + network = FakeNetwork.new(response: nil) + page = FakePage.new(network: network) + browser_with(page: page, profile_token: "profile-token") + same_origin_request = FakeRequest.new(url: "https://example.com/page") + cross_origin_request = FakeRequest.new(url: "https://cdn.example.net/frame") + + page.publish(:request, same_origin_request) + page.publish(:request, cross_origin_request) + + assert_equal [{resource_type: :document}], network.interceptions + assert_equal "profile-token", same_origin_request.continued_headers.fetch("X-Profile-Token") + refute_includes cross_origin_request.continued_headers, "X-Profile-Token" + end + def test_fetch_falls_back_to_page_url_and_original_url page_url_network = FakeNetwork.new(response: nil) page_url = FakePage.new(network: page_url_network, url: "https://example.com/page") @@ -142,14 +193,16 @@ def test_close_allows_missing_browser private - def browser_with(page: FakePage.new(network: FakeNetwork.new(response: nil)), browser: FakeBrowser.new, scroll_page: false) + def browser_with(page: FakePage.new(network: FakeNetwork.new(response: nil)), browser: FakeBrowser.new, profile_token: nil, scroll_page: false) Crawlscope::Browser.allocate.tap do |instance| instance.instance_variable_set(:@base_url, "https://example.com") instance.instance_variable_set(:@timeout_seconds, 20) instance.instance_variable_set(:@network_idle_timeout_seconds, 5) + instance.instance_variable_set(:@profile_token, profile_token) instance.instance_variable_set(:@scroll_page, scroll_page) instance.instance_variable_set(:@browser, browser) instance.instance_variable_set(:@page, page) + instance.send(:configure_profile_requests) end end end diff --git a/test/crawlscope/configuration_test.rb b/test/crawlscope/configuration_test.rb index 48d490c..a7ab6c9 100644 --- a/test/crawlscope/configuration_test.rb +++ b/test/crawlscope/configuration_test.rb @@ -14,6 +14,7 @@ def test_audit_builds_from_configured_callables config.site_name = -> { "Example" } config.concurrency = -> { 4 } config.fetch_executor = -> { :threaded } + config.profile_token = -> { "profile-token" } end audit = Crawlscope.configuration.audit @@ -22,6 +23,7 @@ def test_audit_builds_from_configured_callables assert_equal "/tmp/sitemap.xml", audit.instance_variable_get(:@sitemap_path) assert_equal 4, audit.instance_variable_get(:@concurrency) assert_equal :threaded, audit.instance_variable_get(:@fetch_executor) + assert_equal "profile-token", audit.instance_variable_get(:@profile_token) assert_equal %i[ indexability metadata @@ -53,17 +55,35 @@ def test_audit_raises_without_sitemap_path end def test_defaults_are_normalized - config = Crawlscope::Configuration.new + with_profile_token(nil) do + config = Crawlscope::Configuration.new + + assert_equal [200, 301, 302], config.allowed_statuses + assert_equal 10, config.concurrency + assert_equal :async, config.fetch_executor + assert_equal 4, config.browser_concurrency + assert_equal 5, config.network_idle_timeout_seconds + assert_equal :http, config.renderer + assert_equal 20, config.timeout_seconds + assert_equal $stdout, config.output + assert_nil config.profile_token + assert config.scroll_page? + end + end - assert_equal [200, 301, 302], config.allowed_statuses - assert_equal 10, config.concurrency - assert_equal :async, config.fetch_executor - assert_equal 4, config.browser_concurrency - assert_equal 5, config.network_idle_timeout_seconds - assert_equal :http, config.renderer - assert_equal 20, config.timeout_seconds - assert_equal $stdout, config.output - assert config.scroll_page? + def test_profile_token_defaults_to_the_environment + with_profile_token("environment-profile-token") do + assert_equal "environment-profile-token", Crawlscope::Configuration.new.profile_token + end + end + + def test_configured_profile_token_precedes_the_environment + with_profile_token("environment-profile-token") do + config = Crawlscope::Configuration.new + config.profile_token = "configured-profile-token" + + assert_equal "configured-profile-token", config.profile_token + end end def test_browser_renderer_defaults_to_threaded_fetch_executor @@ -119,4 +139,24 @@ def test_numeric_values_must_be_positive_integers assert_equal "Crawlscope concurrency must be an integer >= 1", error.message end + + private + + def with_profile_token(value) + previous_value = ENV["CRAWLSCOPE_PROFILE_TOKEN"] + + if value.nil? + ENV.delete("CRAWLSCOPE_PROFILE_TOKEN") + else + ENV["CRAWLSCOPE_PROFILE_TOKEN"] = value + end + + yield + ensure + if previous_value.nil? + ENV.delete("CRAWLSCOPE_PROFILE_TOKEN") + else + ENV["CRAWLSCOPE_PROFILE_TOKEN"] = previous_value + end + end end diff --git a/test/crawlscope/crawl_test.rb b/test/crawlscope/crawl_test.rb index 353cebd..842499a 100644 --- a/test/crawlscope/crawl_test.rb +++ b/test/crawlscope/crawl_test.rb @@ -167,6 +167,35 @@ def test_collects_metadata_issues_for_invalid_page ].sort, result.issues.to_a.map(&:code).uniq.sort end + def test_profile_token_reaches_remote_sitemap_and_page_requests + sitemap_request = stub_request(:get, "https://example.com/sitemap.xml") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return( + status: 200, + body: <<~XML + + + https://example.com/about + + XML + ) + page_request = stub_request(:get, "https://example.com/about") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return(status: 200, body: "About") + + Crawlscope::Crawl.new( + base_url: "https://example.com", + sitemap_path: "https://example.com/sitemap.xml", + rules: [], + schema_registry: Crawlscope::SchemaRegistry.default, + fetch_executor: :threaded, + profile_token: "profile-token" + ).call + + assert_requested sitemap_request + assert_requested page_request + end + def test_uses_browser_when_renderer_is_browser File.write( @sitemap_path, diff --git a/test/crawlscope/http_test.rb b/test/crawlscope/http_test.rb index 5e15eb3..c265be1 100644 --- a/test/crawlscope/http_test.rb +++ b/test/crawlscope/http_test.rb @@ -5,13 +5,24 @@ class CrawlscopeHttpTest < Minitest::Test def test_fetch_parses_html_response stub_request(:get, "https://example.com/page") - .to_return(status: 200, headers: {"Content-Type" => "text/html"}, body: "Hello") + .to_return( + status: 200, + headers: { + "Content-Type" => "text/html", + "Server-Timing" => 'db;dur=12.5;desc="Primary database"' + }, + body: "Hello" + ) page = Crawlscope::Http.new(base_url: "https://example.com", timeout_seconds: 2).fetch("https://example.com/page") assert_equal 200, page.status assert page.html? assert_equal "Hello", page.doc.at_css("body").text + timing = page.server_timing + assert_same timing, page.server_timing + assert_equal 12.5, timing.first.duration + assert_equal "Primary database", timing.first.description end def test_fetch_parses_responses_without_content_type_as_html @@ -23,6 +34,54 @@ def test_fetch_parses_responses_without_content_type_as_html assert page.html? end + def test_fetch_sends_profile_token_to_same_origin_requests + request = stub_request(:get, "https://example.com/page") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return(status: 200, body: "") + + Crawlscope::Http.new( + base_url: "https://example.com", + timeout_seconds: 2, + profile_token: "profile-token" + ).fetch("https://example.com/page") + + assert_requested request + end + + def test_fetch_preserves_profile_token_on_same_origin_redirects + stub_request(:get, "https://example.com/start") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return(status: 302, headers: {"Location" => "/final"}) + final_request = stub_request(:get, "https://example.com/final") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return(status: 200, body: "") + + Crawlscope::Http.new( + base_url: "https://example.com", + timeout_seconds: 2, + profile_token: "profile-token" + ).fetch("https://example.com/start") + + assert_requested final_request + end + + def test_fetch_removes_profile_token_from_cross_origin_redirects + stub_request(:get, "https://example.com/start") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return(status: 302, headers: {"Location" => "https://other.example/final"}) + final_request = stub_request(:get, "https://other.example/final") + .with { |request| !request.headers.key?("X-Profile-Token") } + .to_return(status: 200, body: "") + + Crawlscope::Http.new( + base_url: "https://example.com", + timeout_seconds: 2, + profile_token: "profile-token" + ).fetch("https://example.com/start") + + assert_requested final_request + end + def test_fetch_leaves_non_html_response_unparsed stub_request(:get, "https://example.com/feed.xml") .to_return(status: 200, headers: {"content-type" => "application/xml"}, body: "") diff --git a/test/crawlscope/install_generator_test.rb b/test/crawlscope/install_generator_test.rb index 73f9073..896bc82 100644 --- a/test/crawlscope/install_generator_test.rb +++ b/test/crawlscope/install_generator_test.rb @@ -16,11 +16,14 @@ def test_generates_lazy_initializer_and_task_loader Crawlscope::Generators::InstallGenerator.start([], destination_root: destination) initializer = File.join(destination, "config/initializers/crawlscope.rb") + initializer_contents = File.read(initializer) rakefile = File.read(File.join(destination, "Rakefile")) assert File.exist?(initializer) - assert_includes File.read(initializer), "module CrawlscopeConfiguration" - assert_includes File.read(initializer), "CrawlscopeConfiguration.apply if defined?(Crawlscope)" + assert_includes initializer_contents, "module CrawlscopeConfiguration" + assert_includes initializer_contents, "CrawlscopeConfiguration.apply if defined?(Crawlscope)" + assert_includes initializer_contents, "\#{config.base_url.to_s.chomp(\"/\")}/sitemap.xml" + refute_includes initializer_contents, "Rails.public_path" assert_includes rakefile, 'require "crawlscope/tasks"' assert_operator rakefile.index('require "crawlscope/tasks"'), :<, rakefile.index('require_relative "config/application"') @@ -28,14 +31,6 @@ def test_generates_lazy_initializer_and_task_loader assert status.success?, stderr script = <<~RUBY - require "pathname" - - module Rails - def self.public_path - Pathname.new("public") - end - end - require "crawlscope" load ARGV.fetch(0) @@ -43,7 +38,10 @@ def self.public_path CrawlscopeConfiguration.apply abort "configuration replaced" unless Crawlscope.configuration.equal?(configuration) abort "base URL missing" unless configuration.base_url == "http://localhost:3000" - abort "sitemap missing" unless configuration.sitemap_path == "public/sitemap.xml" + abort "sitemap missing" unless configuration.sitemap_path == "http://localhost:3000/sitemap.xml" + + ENV["CRAWLSCOPE_BASE_URL"] = "https://example.com" + abort "configured sitemap missing" unless configuration.sitemap_path == "https://example.com/sitemap.xml" RUBY _stdout, stderr, status = Open3.capture3( RbConfig.ruby, diff --git a/test/crawlscope/reporter_test.rb b/test/crawlscope/reporter_test.rb index 1d7d620..9a4d5c8 100644 --- a/test/crawlscope/reporter_test.rb +++ b/test/crawlscope/reporter_test.rb @@ -10,7 +10,7 @@ def test_reports_ok_result base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com"], - pages: [Object.new], + pages: [page], issues: Crawlscope::IssueCollection.new ) @@ -21,6 +21,7 @@ def test_reports_ok_result assert_includes output, "Crawlscope validation" assert_includes output, "Status: OK" refute_includes output, "Status: FAILED" + refute_includes output, "Server Timing:" end def test_reports_warning_result_with_grouped_one_line_issues @@ -46,7 +47,7 @@ def test_reports_warning_result_with_grouped_one_line_issues base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com/a", "https://example.com/b"], - pages: [Object.new, Object.new], + pages: [page("/one"), page("/two")], issues: issues ) @@ -77,7 +78,7 @@ def test_reports_failed_status_when_errors_are_present base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com/a"], - pages: [Object.new], + pages: [page], issues: issues ) @@ -107,7 +108,7 @@ def test_limits_large_issue_groups base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com"], - pages: [Object.new], + pages: [page], issues: issues ) @@ -137,7 +138,7 @@ def test_reports_ratio_with_enough_precision_to_show_threshold_difference base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com/a"], - pages: [Object.new], + pages: [page], issues: issues ) @@ -164,7 +165,7 @@ def test_reports_source_details_on_one_line base_url: "https://example.com", sitemap_path: "/tmp/sitemap.xml", urls: ["https://example.com"], - pages: [Object.new], + pages: [page], issues: issues ) @@ -176,4 +177,61 @@ def test_reports_source_details_on_one_line assert_includes output, " - /overview-1 indexable internal page is missing from sitemap source: /source-1" assert_includes output, " - /overview-4 indexable internal page is missing from sitemap source: /source-4" end + + def test_reports_server_timing_coverage_percentiles_signals_and_worst_pages + io = StringIO.new + result = Crawlscope::Result.new( + base_url: "https://example.com", + sitemap_path: "/tmp/sitemap.xml", + urls: [ + "https://example.com/fast", + "https://example.com/slow", + "https://example.com/malformed" + ], + pages: [ + page( + "/fast", + server_timing: 'total;dur=100, db;dur=20;desc="Primary", cache;desc="HIT"' + ), + page( + "/slow", + server_timing: 'total;dur=300, db;dur=80, cache;desc="MISS"' + ), + page("/malformed", server_timing: "bad metric;dur=10") + ], + issues: Crawlscope::IssueCollection.new + ) + + Crawlscope::Reporter.new(io: io).report(result) + + output = io.string + assert_includes output, "Server Timing:" + assert_includes output, "Coverage: 3/3 pages (100.0%); durations on 2 pages" + assert_includes output, "Duration metrics (dur; milliseconds assumed):" + assert_includes output, "total: 2 samples / 2 pages; avg 200ms; p50 200ms; p95 290ms; max 300ms" + assert_includes output, 'db "Primary": 1 sample / 1 page' + assert_includes output, 'cache "HIT": 1 sample / 1 page' + assert_includes output, "Worst pages:" + assert_includes output, "/slow: 300ms (total)" + assert_includes output, "Ignored malformed entries: 1" + end + + private + + def page(path = "/", server_timing: nil) + url = "https://example.com#{path}" + headers = {} + headers["Server-Timing"] = server_timing unless server_timing.nil? + + Crawlscope::Page.new( + url: url, + normalized_url: url, + final_url: url, + normalized_final_url: url, + status: 200, + headers: headers, + body: "", + doc: nil + ) + end end diff --git a/test/crawlscope/run_test.rb b/test/crawlscope/run_test.rb index ea9a4c3..47215f7 100644 --- a/test/crawlscope/run_test.rb +++ b/test/crawlscope/run_test.rb @@ -130,29 +130,21 @@ def test_validate_defaults_to_base_url_sitemap_when_not_configured ) end - def test_validate_prefers_local_sitemap_for_localhost + def test_validate_uses_http_sitemap_for_localhost result = FakeResult.new(reported: true) configuration = FakeConfiguration.new(result: result, base_url: "http://localhost:3000", sitemap_path: nil) reporter = FakeReporter.new - tmp_dir = Dir.mktmpdir - sitemap_path = File.join(tmp_dir, "public", "sitemap.xml") - FileUtils.mkdir_p(File.dirname(sitemap_path)) - File.write(sitemap_path, "") - Dir.chdir(tmp_dir) do - Crawlscope::Run.new(configuration: configuration, reporter: reporter).validate - end + Crawlscope::Run.new(configuration: configuration, reporter: reporter).validate assert_equal( { base_url: "http://localhost:3000", - sitemap_path: sitemap_path, + sitemap_path: "http://localhost:3000/sitemap.xml", rule_names: nil }, configuration.received_arguments ) - ensure - FileUtils.rm_rf(tmp_dir) if tmp_dir end def test_validate_json_ld_reports_valid_structured_data diff --git a/test/crawlscope/server_timing_summary_test.rb b/test/crawlscope/server_timing_summary_test.rb new file mode 100644 index 0000000..98a4d8c --- /dev/null +++ b/test/crawlscope/server_timing_summary_test.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require "test_helper" + +class CrawlscopeServerTimingSummaryTest < Minitest::Test + def test_aggregates_coverage_percentiles_signals_and_worst_pages + pages = [ + page("/fast", 'total;dur=100, db;dur=20, cache;desc="HIT"'), + page("/typical", 'total;dur=200, db;dur=50, cache;desc="MISS"'), + page("/slow", 'total;dur=300, db;dur=80, cache;desc="HIT", miss'), + page("/no-header"), + page("/malformed", "bad metric;dur=10") + ] + + summary = Crawlscope::ServerTiming::Summary.new(pages) + + assert summary.present? + assert_equal 5, summary.pages.size + assert_equal 4, summary.coverage + assert_equal 3, summary.duration_pages + assert_equal 1, summary.invalid_count + + total = summary.metrics.find { |metric| metric[:name] == "total" } + assert_equal 3, total[:samples] + assert_equal 3, total[:pages] + assert_in_delta 200.0, total[:average] + assert_in_delta 200.0, total[:p50] + assert_in_delta 290.0, total[:p95] + assert_in_delta 300.0, total[:maximum] + + hit = summary.signals.find do |signal| + signal[:name] == "cache" && signal[:description] == "HIT" + end + assert_equal 2, hit[:samples] + assert_equal 2, hit[:pages] + + assert_equal( + [ + ["https://example.com/slow", "total", 300.0], + ["https://example.com/typical", "total", 200.0] + ], + summary.worst_pages(limit: 2).map do |sample| + [sample[:page].url, sample[:metric].name, sample[:metric].duration] + end + ) + end + + def test_is_absent_when_no_pages_publish_the_header + summary = Crawlscope::ServerTiming::Summary.new([page("/one"), page("/two")]) + + refute summary.present? + assert_empty summary.metrics + assert_empty summary.signals + assert_empty summary.worst_pages + end + + def test_returns_ten_worst_pages_by_default + pages = 12.times.map do |index| + page("/page-#{index + 1}", "total;dur=#{index + 1}") + end + + summary = Crawlscope::ServerTiming::Summary.new(pages) + + assert_equal 10, summary.worst_pages.size + assert_equal "https://example.com/page-12", summary.worst_pages.first[:page].url + assert_equal "https://example.com/page-3", summary.worst_pages.last[:page].url + end + + private + + def page(path, server_timing = nil) + url = "https://example.com#{path}" + headers = {} + headers["Server-Timing"] = server_timing unless server_timing.nil? + + Crawlscope::Page.new( + url: url, + normalized_url: url, + final_url: url, + normalized_final_url: url, + status: 200, + headers: headers, + body: "", + doc: nil + ) + end +end diff --git a/test/crawlscope/server_timing_test.rb b/test/crawlscope/server_timing_test.rb new file mode 100644 index 0000000..bf142f0 --- /dev/null +++ b/test/crawlscope/server_timing_test.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require "test_helper" + +class CrawlscopeServerTimingTest < Minitest::Test + def test_parses_metrics_durations_descriptions_and_unknown_parameters + timing = Crawlscope::ServerTiming.new( + 'miss, db;dur=53, cache;desc="Cache Read";dur=23.2;region=atl' + ) + + assert timing.present? + assert_equal 0, timing.invalid_count + assert_equal %w[miss db cache], timing.map(&:name) + + cache = timing.to_a.last + assert_equal 23.2, cache.duration + assert_equal "Cache Read", cache.description + end + + def test_preserves_duplicate_metrics_and_uses_the_first_duplicate_parameter + timing = Crawlscope::ServerTiming.new( + ["db;DUR=10;dur=20", "db;dur=30"] + ) + + assert_equal [10.0, 30.0], timing.map(&:duration) + assert_equal 0, timing.invalid_count + end + + def test_parses_rails_notification_names + timing = Crawlscope::ServerTiming.new( + "sql.active_record;dur=82.4, " \ + "render_template.action_view;dur=10.2, " \ + "process_action.action_controller;dur=101.5" + ) + + assert_equal( + %w[ + sql.active_record + render_template.action_view + process_action.action_controller + ], + timing.map(&:name) + ) + assert_equal 0, timing.invalid_count + end + + def test_handles_delimiters_and_escapes_inside_quoted_descriptions + timing = Crawlscope::ServerTiming.new( + 'cache;desc="Cache, read; \\"fast\\"";dur=23.2' + ) + + assert_equal 1, timing.size + assert_equal 'Cache, read; "fast"', timing.first.description + assert_equal 0, timing.invalid_count + end + + def test_reports_invalid_entries_without_discarding_valid_metrics + timing = Crawlscope::ServerTiming.new( + "bad metric;dur=2, db;dur=nope, app;dur=3, cache;broken" + ) + + assert_equal %w[app cache], timing.map(&:name) + assert_equal 3.0, timing.first.duration + assert_equal 2, timing.invalid_count + end + + def test_rejects_malformed_standard_parameters + timing = Crawlscope::ServerTiming.new( + "db;dur, cache;desc=not quoted, app;dur=3" + ) + + assert_equal ["app"], timing.map(&:name) + assert_equal 2, timing.invalid_count + end + + def test_keeps_complete_metrics_before_an_unterminated_description + timing = Crawlscope::ServerTiming.new( + 'db;dur=5, cache;desc="unterminated' + ) + + assert_equal ["db"], timing.map(&:name) + assert_equal 1, timing.invalid_count + end + + def test_ignores_empty_list_members + timing = Crawlscope::ServerTiming.new(", db;dur=5, , app;dur=3,") + + assert_equal %w[db app], timing.map(&:name) + assert_equal 0, timing.invalid_count + end + + def test_distinguishes_an_absent_header_from_an_empty_header + refute Crawlscope::ServerTiming.new(nil).present? + + timing = Crawlscope::ServerTiming.new("") + + assert timing.present? + assert_empty timing + assert_equal 0, timing.invalid_count + end +end diff --git a/test/crawlscope/sitemap_test.rb b/test/crawlscope/sitemap_test.rb index ca83218..74bfe6d 100644 --- a/test/crawlscope/sitemap_test.rb +++ b/test/crawlscope/sitemap_test.rb @@ -34,6 +34,27 @@ def test_parses_remote_sitemap_urlset assert_equal ["https://www.example.com/", "https://www.example.com/pricing"], parser.urls(base_url: "https://www.example.com") end + def test_remote_sitemap_sends_profile_token + request = stub_request(:get, "https://www.example.com/sitemap.xml") + .with(headers: {"X-Profile-Token" => "profile-token"}) + .to_return( + status: 200, + body: <<~XML + + + https://www.example.com/ + + XML + ) + + Crawlscope::Sitemap.new( + path: "https://www.example.com/sitemap.xml", + profile_token: "profile-token" + ).urls(base_url: "https://www.example.com") + + assert_requested request + end + def test_parses_remote_sitemap_index_with_child_sitemap stub_request(:get, "https://www.example.com/sitemap.xml") .to_return(