diff options
| -rw-r--r-- | README.org | 2 | ||||
| -rw-r--r-- | tests/test-wttrin--display-weather.el | 6 | ||||
| -rw-r--r-- | tests/test-wttrin--get-cached-or-fetch.el | 403 | ||||
| -rw-r--r-- | tests/test-wttrin-ansi-color-rendering.el | 416 | ||||
| -rw-r--r-- | tests/test-wttrin-mode-initialization-order.el | 98 | ||||
| -rw-r--r-- | wttrin.el | 30 |
6 files changed, 936 insertions, 19 deletions
@@ -88,7 +88,7 @@ Simply use the keybinding you assigned, or run `M-x wttrin` to display the weath [[assets/location-menu.png]] -Choose one, or for a quick one-time weather check, type a new location and ⏎ . After the weather is displayed, you can press `g` to check another location or `q` to exit, just like it says in the buffer. +Choose one, or for a quick one-time weather check, type a new location and ⏎ . After the weather is displayed, you can press `a` to check another location, `g` to refresh, or `q` to quit. ** Customization Wttrin can be customized using the built-in Emacs Customize interface. To do this, type M-x customize ⏎ wttrin ⏎ and use the UI. However, it's more portable and reproducible to keep the customizations in your init file, so do that. diff --git a/tests/test-wttrin--display-weather.el b/tests/test-wttrin--display-weather.el index 364908f..4b2a90d 100644 --- a/tests/test-wttrin--display-weather.el +++ b/tests/test-wttrin--display-weather.el @@ -87,7 +87,7 @@ Weather report: Paris, France ;; Check that keybindings are set (they should be in the local map) (should (keymapp (current-local-map))) (should (commandp (lookup-key (current-local-map) "q"))) - (should (commandp (lookup-key (current-local-map) "r"))) + (should (commandp (lookup-key (current-local-map) "a"))) (should (commandp (lookup-key (current-local-map) "g"))))) (test-wttrin--display-weather-teardown))) @@ -103,8 +103,8 @@ Weather report: Paris, France (forward-line -2) ;; Should contain help text (should (search-forward "Press:" nil t)) - (should (search-forward "[g] to query another location" nil t)) - (should (search-forward "[r] to refresh" nil t)) + (should (search-forward "[a] for another location" nil t)) + (should (search-forward "[g] to refresh" nil t)) (should (search-forward "[q] to quit" nil t)))) (test-wttrin--display-weather-teardown))) diff --git a/tests/test-wttrin--get-cached-or-fetch.el b/tests/test-wttrin--get-cached-or-fetch.el new file mode 100644 index 0000000..77a2689 --- /dev/null +++ b/tests/test-wttrin--get-cached-or-fetch.el @@ -0,0 +1,403 @@ +;;; test-wttrin--get-cached-or-fetch.el --- Tests for wttrin--get-cached-or-fetch -*- lexical-binding: t; -*- + +;; Copyright (C) 2025 Craig Jennings + +;;; Commentary: + +;; Unit tests for wttrin--get-cached-or-fetch function. +;; Tests the core cache workflow: cache hits, misses, expiration, and error fallback. + +;;; Code: + +(require 'ert) +(require 'wttrin) +(require 'testutil-wttrin) + +;;; Test Data Fixtures + +(defconst test-wttrin--get-cached-or-fetch-sample-weather + "Weather for Paris: Sunny 22°C" + "Sample weather data for testing.") + +(defconst test-wttrin--get-cached-or-fetch-new-weather + "Weather for Paris: Cloudy 18°C" + "Updated weather data for testing cache updates.") + +;;; Test Setup and Teardown + +(defun test-wttrin--get-cached-or-fetch-setup () + "Setup for get-cached-or-fetch tests." + (testutil-wttrin-setup)) + +(defun test-wttrin--get-cached-or-fetch-teardown () + "Teardown for get-cached-or-fetch tests." + (testutil-wttrin-teardown)) + +;;; Normal Cases + +(ert-deftest test-wttrin--get-cached-or-fetch-normal-cache-hit-returns-cached-data () + "Test that fresh cached data is returned without fetching." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Paris") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (callback-result nil) + (fetch-called nil)) + ;; Pre-populate cache with fresh data + (puthash cache-key (cons now test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + ;; Mock time to be 100 seconds later (well within TTL of 900) + (cl-letf (((symbol-function 'float-time) + (lambda () (+ now 100.0))) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location _callback) + (setq fetch-called t)))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should return cached data immediately + (should (equal callback-result test-wttrin--get-cached-or-fetch-sample-weather)) + ;; Should NOT call fetch + (should-not fetch-called))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-normal-cache-miss-fetches-new-data () + "Test that missing cache entry triggers fetch and stores result." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "London") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (callback-result nil) + (fetch-called nil)) + + (cl-letf (((symbol-function 'float-time) + (lambda () now)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should call fetch + (should fetch-called) + ;; Should return fresh data + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)) + ;; Should store in cache + (let ((cached (gethash cache-key wttrin--cache))) + (should cached) + (should (equal (car cached) now)) + (should (equal (cdr cached) test-wttrin--get-cached-or-fetch-new-weather))))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-normal-expired-cache-fetches-new-data () + "Test that expired cache triggers fetch and updates cache." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Tokyo") + (cache-key (wttrin--make-cache-key location)) + (old-time 1000.0) + (new-time (+ old-time 1000.0)) ; 1000 seconds later (> 900 TTL) + (callback-result nil) + (fetch-called nil)) + + ;; Pre-populate cache with old data + (puthash cache-key (cons old-time test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () new-time)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should call fetch due to expiration + (should fetch-called) + ;; Should return new data + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)) + ;; Should update cache timestamp + (let ((cached (gethash cache-key wttrin--cache))) + (should (equal (car cached) new-time)) + (should (equal (cdr cached) test-wttrin--get-cached-or-fetch-new-weather))))) + (test-wttrin--get-cached-or-fetch-teardown))) + +;;; Boundary Cases + +(ert-deftest test-wttrin--get-cached-or-fetch-boundary-exactly-at-ttl-fetches () + "Test that cache exactly at TTL boundary triggers fetch." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Berlin") + (cache-key (wttrin--make-cache-key location)) + (old-time 1000.0) + ;; Exactly at TTL boundary (900 seconds = wttrin-cache-ttl) + (new-time (+ old-time wttrin-cache-ttl)) + (callback-result nil) + (fetch-called nil)) + + ;; Pre-populate cache + (puthash cache-key (cons old-time test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () new-time)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; At exactly TTL, should fetch (not <) + (should fetch-called) + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-boundary-one-second-before-ttl-uses-cache () + "Test that cache one second before TTL uses cached data." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Madrid") + (cache-key (wttrin--make-cache-key location)) + (old-time 1000.0) + ;; One second before TTL expiration + (new-time (+ old-time (- wttrin-cache-ttl 1))) + (callback-result nil) + (fetch-called nil)) + + ;; Pre-populate cache + (puthash cache-key (cons old-time test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () new-time)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location _callback) + (setq fetch-called t)))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should use cache (still fresh) + (should-not fetch-called) + (should (equal callback-result test-wttrin--get-cached-or-fetch-sample-weather)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-boundary-force-refresh-bypasses-fresh-cache () + "Test that force refresh flag bypasses fresh cache." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Rome") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (callback-result nil) + (fetch-called nil) + (wttrin--force-refresh t)) ; Force refresh enabled + + ;; Pre-populate cache with fresh data + (puthash cache-key (cons now test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () (+ now 100.0))) ; Well within TTL + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should fetch despite fresh cache + (should fetch-called) + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-boundary-empty-cache-fetches () + "Test that completely empty cache triggers fetch." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Athens") + (callback-result nil) + (fetch-called nil)) + + ;; Cache is already empty from setup + (should (= (hash-table-count wttrin--cache) 0)) + + (cl-letf (((symbol-function 'float-time) + (lambda () 1000.0)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should fetch + (should fetch-called) + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-boundary-cache-key-includes-unit-system () + "Test that cache keys differentiate by unit system." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Oslo") + (now 1000.0) + (metric-data "Weather: 20°C") + (imperial-data "Weather: 68°F") + (callback-result nil)) + + ;; Cache metric version + (let ((wttrin-unit-system "m")) + (puthash (wttrin--make-cache-key location) + (cons now metric-data) + wttrin--cache)) + + ;; Cache imperial version + (let ((wttrin-unit-system "u")) + (puthash (wttrin--make-cache-key location) + (cons now imperial-data) + wttrin--cache)) + + (cl-letf (((symbol-function 'float-time) + (lambda () (+ now 100.0)))) + + ;; Fetch with metric - should get metric cache + (let ((wttrin-unit-system "m")) + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + (should (equal callback-result metric-data))) + + ;; Fetch with imperial - should get imperial cache + (let ((wttrin-unit-system "u")) + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + (should (equal callback-result imperial-data))))) + (test-wttrin--get-cached-or-fetch-teardown))) + +;;; Error Cases + +(ert-deftest test-wttrin--get-cached-or-fetch-error-fetch-fails-with-stale-cache-returns-stale () + "Test that fetch failure with stale cache falls back to stale data." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Vienna") + (cache-key (wttrin--make-cache-key location)) + (old-time 1000.0) + (new-time (+ old-time 2000.0)) ; Well expired + (callback-result nil) + (message-shown nil)) + + ;; Pre-populate cache with expired data + (puthash cache-key (cons old-time test-wttrin--get-cached-or-fetch-sample-weather) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () new-time)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + ;; Simulate network failure + (funcall callback nil))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil)) + ((symbol-function 'message) + (lambda (format-string &rest _args) + (setq message-shown format-string)))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should fall back to stale cache + (should (equal callback-result test-wttrin--get-cached-or-fetch-sample-weather)) + ;; Should show message about using cached version + (should message-shown) + (should (string-match-p "cached" message-shown)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-error-fetch-fails-with-no-cache-returns-nil () + "Test that fetch failure with no cache returns nil." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location "Dublin") + (callback-result 'unset)) + + ;; Cache is empty + (should (= (hash-table-count wttrin--cache) 0)) + + (cl-letf (((symbol-function 'float-time) + (lambda () 1000.0)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + ;; Simulate network failure + (funcall callback nil))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should return nil (no fallback available) + (should (null callback-result)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(ert-deftest test-wttrin--get-cached-or-fetch-error-nil-location-creates-cache-key () + "Test that nil location is handled (creates cache key with nil)." + (test-wttrin--get-cached-or-fetch-setup) + (unwind-protect + (let* ((location nil) + (callback-result nil) + (fetch-called nil)) + + (cl-letf (((symbol-function 'float-time) + (lambda () 1000.0)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-called t) + (funcall callback test-wttrin--get-cached-or-fetch-new-weather))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + (wttrin--get-cached-or-fetch + location + (lambda (data) (setq callback-result data))) + + ;; Should attempt to fetch (nil is a valid location input) + (should fetch-called) + (should (equal callback-result test-wttrin--get-cached-or-fetch-new-weather)))) + (test-wttrin--get-cached-or-fetch-teardown))) + +(provide 'test-wttrin--get-cached-or-fetch) +;;; test-wttrin--get-cached-or-fetch.el ends here diff --git a/tests/test-wttrin-ansi-color-rendering.el b/tests/test-wttrin-ansi-color-rendering.el new file mode 100644 index 0000000..0945a2f --- /dev/null +++ b/tests/test-wttrin-ansi-color-rendering.el @@ -0,0 +1,416 @@ +;;; test-wttrin-ansi-color-rendering.el --- Test ANSI color rendering -*- lexical-binding: t; -*- + +;; Copyright (C) 2025 Craig Jennings + +;;; Commentary: + +;; Test that ANSI color codes are properly rendered in the weather buffer. +;; This reproduces the bug where clicking the mode-line icon shows white text. + +;;; Code: + +(require 'ert) +(require 'wttrin) +(require 'xterm-color) +(require 'testutil-wttrin) + +;;; Test Data - Realistic wttr.in response with ANSI codes + +(defconst test-wttrin-ansi-sample-with-colors + "Weather report: Paris + +\x1b[38;5;226m \\ /\x1b[0m Partly cloudy +\x1b[38;5;226m _ /\"\"\x1b[38;5;250m.-.\x1b[0m \x1b[38;5;118m+13\x1b[0m(\x1b[38;5;082m12\x1b[0m) °C +\x1b[38;5;226m \\_\x1b[38;5;250m( ). \x1b[0m ↑ \x1b[38;5;190m12\x1b[0m km/h +" + "Sample weather data with ANSI color codes (escape sequences).") + +;;; Test Setup and Teardown + +(defun test-wttrin-ansi-setup () + "Setup for ANSI color rendering tests." + (testutil-wttrin-setup) + (when (get-buffer "*wttr.in*") + (kill-buffer "*wttr.in*"))) + +(defun test-wttrin-ansi-teardown () + "Teardown for ANSI color rendering tests." + (testutil-wttrin-teardown) + (when (get-buffer "*wttr.in*") + (kill-buffer "*wttr.in*"))) + +;;; Helper Functions + +(defun test-wttrin-ansi--count-colored-text () + "Count number of characters with color text properties in current buffer. +Returns cons (colored-chars . total-chars)." + (save-excursion + (goto-char (point-min)) + (let ((colored 0) + (total 0)) + (while (not (eobp)) + (let ((props (text-properties-at (point)))) + (setq total (1+ total)) + ;; Check for xterm-color face properties + (when (or (plist-get props 'face) + (plist-get props 'font-lock-face)) + (setq colored (1+ colored)))) + (forward-char 1)) + (cons colored total)))) + +(defun test-wttrin-ansi--has-ansi-escape-codes (str) + "Check if STR contains ANSI escape codes." + (string-match-p "\x1b\\[" str)) + +;;; Tests + +(ert-deftest test-wttrin-ansi-xterm-color-filter-processes-codes () + "Test that xterm-color-filter properly processes ANSI codes." + (test-wttrin-ansi-setup) + (unwind-protect + (let ((filtered (xterm-color-filter test-wttrin-ansi-sample-with-colors))) + ;; After filtering, ANSI escape codes should be removed + (should-not (test-wttrin-ansi--has-ansi-escape-codes filtered)) + + ;; The filtered text should be shorter (escape codes removed) + (should (< (length filtered) (length test-wttrin-ansi-sample-with-colors))) + + ;; Text should still contain the actual weather content + (should (string-match-p "Paris" filtered)) + (should (string-match-p "Partly cloudy" filtered))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-display-weather-renders-colors () + "Test that display-weather properly renders ANSI colors in buffer." + (test-wttrin-ansi-setup) + (unwind-protect + (progn + (wttrin--display-weather "Paris" test-wttrin-ansi-sample-with-colors) + + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; Buffer should not contain raw ANSI escape codes + (let ((buffer-text (buffer-substring-no-properties (point-min) (point-max)))) + (should-not (test-wttrin-ansi--has-ansi-escape-codes buffer-text))) + + ;; Buffer should contain the weather text + (goto-char (point-min)) + (should (search-forward "Paris" nil t)) + (should (search-forward "Partly cloudy" nil t)) + + ;; Check that some text has color properties + (let* ((counts (test-wttrin-ansi--count-colored-text)) + (colored (car counts)) + (total (cdr counts))) + ;; At least some characters should have color properties + ;; (not all white text) + (should (> colored 0)) + + ;; Colored text should be a reasonable portion (not just 2 chars) + ;; With ANSI codes, we expect at least 10% of text to be colored + (should (> colored (/ total 10)))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-mode-line-click-renders-colors () + "Test that clicking mode-line icon renders colors properly. +This reproduces the bug where mode-line click shows white text." + (test-wttrin-ansi-setup) + (unwind-protect + (let* ((location "Paris") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (wttrin-mode-line-favorite-location location)) + + ;; Mock the async fetch to return ANSI-coded data + (cl-letf (((symbol-function 'float-time) + (lambda () now)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (funcall callback test-wttrin-ansi-sample-with-colors))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + ;; Simulate mode-line click by calling wttrin + ;; (wttrin-mode-line-click just calls this) + (wttrin location) + + ;; Give async callback time to execute + ;; (In real execution this is async, but our mock is synchronous) + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; Buffer should not contain raw ANSI codes + (let ((buffer-text (buffer-substring-no-properties (point-min) (point-max)))) + (should-not (test-wttrin-ansi--has-ansi-escape-codes buffer-text))) + + ;; Check that text has color properties (not all white) + (let* ((counts (test-wttrin-ansi--count-colored-text)) + (colored (car counts))) + ;; Should have colored text (proving colors are rendered) + (should (> colored 0)))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-cached-data-preserves-colors () + "Test that cached weather data preserves color information. +Verifies that cache doesn't strip ANSI codes or color properties." + (test-wttrin-ansi-setup) + (unwind-protect + (let* ((location "Berlin") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0)) + + ;; Pre-populate cache with ANSI-coded data + (puthash cache-key (cons now test-wttrin-ansi-sample-with-colors) + wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () (+ now 100.0)))) ; Within TTL + + ;; Call wttrin - should use cached data + (wttrin location) + + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; Even with cached data, colors should be rendered + (let* ((counts (test-wttrin-ansi--count-colored-text)) + (colored (car counts)) + (total (cdr counts))) + (should (> colored 0)) + ;; Should have reasonable color coverage + (should (> colored (/ total 10))))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-buffer-face-mode-doesnt-strip-colors () + "Test that buffer-face-mode doesn't strip xterm-color face properties. +This reproduces the bug where weather buffer shows mostly white text." + (test-wttrin-ansi-setup) + (unwind-protect + (progn + (wttrin--display-weather "Paris" test-wttrin-ansi-sample-with-colors) + + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; Check that face-remapping is active (for font customization) + (should face-remapping-alist) + + ;; But colors should still be present despite face remapping + (let* ((counts (test-wttrin-ansi--count-colored-text)) + (colored (car counts)) + (total (cdr counts)) + (percentage (if (> total 0) + (* 100.0 (/ (float colored) total)) + 0))) + ;; Should have substantial colored text (>10%) + ;; If this fails with ~5% or less, buffer-face-mode is interfering + (should (> percentage 10.0)) + + ;; With face-remap-add-relative fix, we get ~30-40 colored chars + ;; (our test data is small, so absolute count is low) + ;; The key is the percentage, not absolute count + (should (> colored 20))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-mode-line-doesnt-pollute-main-cache () + "Test that mode-line weather fetch doesn't pollute main cache with plain text. +This reproduces the bug where clicking mode-line shows white text." + (test-wttrin-ansi-setup) + (unwind-protect + (let* ((location "Berlin") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (plain-text-weather "Berlin: ☀️ +20°C Clear") ; No ANSI codes + (ansi-weather test-wttrin-ansi-sample-with-colors)) ; Has ANSI codes + + ;; Simulate mode-line storing plain-text in cache (the bug) + ;; This shouldn't happen, but let's verify the system is resilient + (puthash cache-key (cons now plain-text-weather) wttrin--cache) + + (cl-letf (((symbol-function 'float-time) + (lambda () (+ now 100.0))) ; Within TTL, will use cache + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + ;; Should never be called since cache is fresh + (error "Should not fetch when cache is fresh")))) + + ;; Call wttrin - it will use cached data + (wttrin location) + + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; Even if cache has plain text, should we handle it gracefully? + ;; At minimum, buffer should exist and not error + (should (> (buffer-size) 0)) + + ;; The real fix: mode-line should use separate cache or namespace + ;; For now, document that plain-text cache = no colors + (let* ((buffer-text (buffer-substring-no-properties (point-min) (point-max)))) + ;; Should contain the weather data (even if not colored) + (should (string-match-p "Berlin" buffer-text)))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-full-scenario-mode-line-then-click () + "Test full scenario: mode-line fetch, then user clicks to open buffer. +This is the exact user workflow that exposes the bug." + (test-wttrin-ansi-setup) + (unwind-protect + (let* ((location "Tokyo") + (now 1000.0) + (wttrin-mode-line-favorite-location location) + (mode-line-fetch-count 0) + (main-fetch-count 0)) + + (cl-letf (((symbol-function 'float-time) + (lambda () now)) + ((symbol-function 'url-retrieve) + (lambda (url callback) + ;; Mode-line uses url-retrieve directly + (setq mode-line-fetch-count (1+ mode-line-fetch-count)) + ;; Simulate async callback with plain-text format + (with-temp-buffer + (insert "HTTP/1.1 200 OK\n\n") + (insert "Tokyo: ☀️ +25°C Clear") ; Plain text, no ANSI + (funcall callback nil)))) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + ;; Main buffer fetch should get ANSI codes + (setq main-fetch-count (1+ main-fetch-count)) + (funcall callback test-wttrin-ansi-sample-with-colors))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + ;; Step 1: Mode-line fetches weather (simulated) + (wttrin--mode-line-fetch-weather) + ;; Mode-line should have fetched + (should (= mode-line-fetch-count 1)) + + ;; Step 2: User clicks mode-line icon (calls wttrin) + (wttrin location) + ;; Main fetch should have happened (cache miss) + (should (= main-fetch-count 1)) + + ;; Step 3: Verify buffer has colors + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + (let* ((counts (test-wttrin-ansi--count-colored-text)) + (colored (car counts)) + (total (cdr counts))) + ;; Should have colored text + (should (> colored 0)) + ;; Should be substantial (>10%) + (should (> colored (/ total 10))))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-cache-stores-ansi-codes () + "Test that cache stores raw ANSI codes, not filtered text. +This verifies the cache workflow preserves ANSI codes for later filtering." + (test-wttrin-ansi-setup) + (unwind-protect + (let* ((location "Vienna") + (cache-key (wttrin--make-cache-key location)) + (now 1000.0) + (fetch-count 0)) + + (cl-letf (((symbol-function 'float-time) + (lambda () now)) + ((symbol-function 'wttrin-fetch-raw-string) + (lambda (_location callback) + (setq fetch-count (1+ fetch-count)) + ;; Simulate fetch returning ANSI codes + (funcall callback test-wttrin-ansi-sample-with-colors))) + ((symbol-function 'wttrin--cleanup-cache-if-needed) + (lambda () nil))) + + ;; Fetch weather (will cache the result) + (wttrin location) + + ;; Verify fetch was called + (should (= fetch-count 1)) + + ;; Check what's in the cache + (let* ((cached (gethash cache-key wttrin--cache)) + (cached-data (cdr cached))) + + ;; Cache should have data + (should cached) + (should cached-data) + + ;; CRITICAL: Cache should store RAW ANSI codes + ;; NOT filtered text with face properties + (should (stringp cached-data)) + (should (string-match-p "\x1b\\[" cached-data)) + + ;; Verify it's the original ANSI string + (should (string-match-p "Partly cloudy" cached-data))))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-fetch-returns-ansi-codes () + "Test that wttrin-fetch-raw-string returns data with ANSI codes. +This verifies the fetch function returns unfiltered data from wttr.in." + (test-wttrin-ansi-setup) + (unwind-protect + (let ((location "Prague") + (callback-data nil) + (url-retrieve-called nil) + (wttrin-unit-system "u")) ; Set unit system for URL generation + + ;; Mock url-retrieve to simulate wttr.in response + (cl-letf (((symbol-function 'url-retrieve) + (lambda (url callback) + (setq url-retrieve-called t) + ;; Verify URL has ANSI format flag + ;; wttr.in uses ?mA, ?uA, or ?A for ANSI colored output + (should (or (string-match-p "\\?mA" url) + (string-match-p "\\?uA" url) + (string-match-p "\\?A" url))) + ;; Simulate HTTP response with ANSI codes + (with-temp-buffer + (insert "HTTP/1.1 200 OK\n\n") + (insert test-wttrin-ansi-sample-with-colors) + (funcall callback nil))))) + + ;; Call fetch + (wttrin-fetch-raw-string + location + (lambda (data) + (setq callback-data data))) + + ;; Verify url-retrieve was called + (should url-retrieve-called) + + ;; Verify callback received ANSI codes + (should callback-data) + (should (string-match-p "\x1b\\[" callback-data)))) + (test-wttrin-ansi-teardown))) + +(ert-deftest test-wttrin-ansi-build-url-includes-ansi-flag () + "Test that wttrin--build-url includes ANSI color flag in URL. +The 'A' flag tells wttr.in to include ANSI color codes in the response." + (test-wttrin-ansi-setup) + (unwind-protect + (progn + ;; Test with metric system + (let ((wttrin-unit-system "m")) + (let ((url (wttrin--build-url "Berlin"))) + ;; Should have ?mA flag (metric + ANSI) + (should (string-match-p "\\?mA" url)))) + + ;; Test with USCS system + (let ((wttrin-unit-system "u")) + (let ((url (wttrin--build-url "London"))) + ;; Should have ?uA flag (USCS + ANSI) + (should (string-match-p "\\?uA" url)))) + + ;; Test with no unit system + (let ((wttrin-unit-system nil)) + (let ((url (wttrin--build-url "Paris"))) + ;; Should have just ?A flag (ANSI) + (should (string-match-p "\\?A" url))))) + (test-wttrin-ansi-teardown))) + +(provide 'test-wttrin-ansi-color-rendering) +;;; test-wttrin-ansi-color-rendering.el ends here diff --git a/tests/test-wttrin-mode-initialization-order.el b/tests/test-wttrin-mode-initialization-order.el new file mode 100644 index 0000000..20adf06 --- /dev/null +++ b/tests/test-wttrin-mode-initialization-order.el @@ -0,0 +1,98 @@ +;;; test-wttrin-mode-initialization-order.el --- Test mode initialization order -*- lexical-binding: t; -*- + +;; Copyright (C) 2025 Craig Jennings + +;;; Commentary: + +;; This test verifies that wttrin--display-weather initializes wttrin-mode +;; BEFORE setting buffer-local variables. This prevents kill-all-local-variables +;; (called by derived modes) from wiping out important state like xterm-color--state. +;; +;; Bug context: On fresh Emacs launch, weather displayed with no colors because +;; xterm-color--state was set buffer-local BEFORE wttrin-mode was called, and +;; wttrin-mode's kill-all-local-variables wiped it out. + +;;; Code: + +(require 'ert) +(require 'wttrin) +(require 'testutil-wttrin) + +(defun test-wttrin-mode-init-setup () + "Setup for mode initialization tests." + (testutil-wttrin-setup) + (when (get-buffer "*wttr.in*") + (kill-buffer "*wttr.in*"))) + +(defun test-wttrin-mode-init-teardown () + "Teardown for mode initialization tests." + (testutil-wttrin-teardown) + (when (get-buffer "*wttr.in*") + (kill-buffer "*wttr.in*"))) + +(ert-deftest test-wttrin-mode-initialization-order-mode-before-buffer-local-vars () + "Test that wttrin-mode is activated before setting buffer-local variables. + +This test verifies the fix for the color rendering bug where xterm-color--state +was wiped by kill-all-local-variables. + +The test strategy: +1. Advise wttrin-mode to record when it's called +2. Advise setq-local to record when buffer-local vars are set +3. Call wttrin--display-weather +4. Verify wttrin-mode was called BEFORE any buffer-local vars were set" + (test-wttrin-mode-init-setup) + (unwind-protect + (let ((mode-called-at nil) + (first-setq-local-at nil) + (call-counter 0)) + + ;; Advise to track when wttrin-mode is called + (cl-letf (((symbol-function 'wttrin-mode) + (let ((orig-fn (symbol-function 'wttrin-mode))) + (lambda () + (setq mode-called-at (cl-incf call-counter)) + (funcall orig-fn)))) + + ;; Advise to track first buffer-local variable set + ((symbol-function 'set) + (let ((orig-fn (symbol-function 'set))) + (lambda (symbol value) + ;; Track xterm-color--state specifically + (when (and (eq symbol 'xterm-color--state) + (null first-setq-local-at)) + (setq first-setq-local-at (cl-incf call-counter))) + (funcall orig-fn symbol value))))) + + (wttrin--display-weather "Paris" "Test weather data") + + ;; Verify mode was called + (should mode-called-at) + ;; Verify buffer-local var was set + (should first-setq-local-at) + ;; Critical: mode must be called BEFORE buffer-local var + (should (< mode-called-at first-setq-local-at)))) + + (test-wttrin-mode-init-teardown))) + +(ert-deftest test-wttrin-mode-initialization-order-xterm-color-state-survives () + "Test that xterm-color--state remains buffer-local after wttrin--display-weather. + +This verifies that the state isn't wiped by kill-all-local-variables." + (test-wttrin-mode-init-setup) + (unwind-protect + (progn + (wttrin--display-weather "London" "Test data") + + (should (get-buffer "*wttr.in*")) + + (with-current-buffer "*wttr.in*" + ;; xterm-color--state should be buffer-local + (should (local-variable-p 'xterm-color--state)) + ;; It should have the correct value + (should (eq xterm-color--state :char)))) + + (test-wttrin-mode-init-teardown))) + +(provide 'test-wttrin-mode-initialization-order) +;;; test-wttrin-mode-initialization-order.el ends here @@ -230,8 +230,8 @@ CALLBACK is called with the weather data string when ready, or nil on error." (defvar wttrin-mode-map (let ((map (make-sparse-keymap))) - (define-key map (kbd "g") 'wttrin-requery) - (define-key map (kbd "r") 'wttrin-requery-force) + (define-key map (kbd "a") 'wttrin-requery) + (define-key map (kbd "g") 'wttrin-requery-force) ;; Note: 'q' is bound to quit-window by special-mode map) "Keymap for wttrin-mode.") @@ -243,9 +243,10 @@ Weather data is displayed in a read-only buffer with the following keybindings: \\{wttrin-mode-map}" (buffer-disable-undo) - (setq buffer-face-mode-face `(:family ,wttrin-font-name - :height ,wttrin-font-height)) - (buffer-face-mode t)) + ;; Use face-remap instead of buffer-face-mode to preserve xterm-color faces + (face-remap-add-relative 'default + :family wttrin-font-name + :height wttrin-font-height)) (defun wttrin--save-debug-data (location-name raw-string) "Save RAW-STRING to a timestamped debug file for LOCATION-NAME. @@ -273,9 +274,15 @@ Returns the path to the saved file." (let ((buffer (get-buffer-create (format "*wttr.in*")))) (switch-to-buffer buffer) - ;; Temporarily allow editing (in case mode is already active) + ;; Enable wttrin-mode first (calls kill-all-local-variables) + ;; This must be done before setting any buffer-local variables + (wttrin-mode) + + ;; Temporarily allow editing (let ((inhibit-read-only t)) (erase-buffer) + ;; Initialize xterm-color state AFTER wttrin-mode to prevent it being wiped + (setq-local xterm-color--state :char) (insert (xterm-color-filter raw-string)) ;; Remove verbose Location: coordinate line @@ -285,24 +292,17 @@ Returns the path to the saved file." ;; Add user instructions at the bottom (goto-char (point-max)) - (insert "\n\nPress: [g] to query another location [r] to refresh [q] to quit") + (insert "\n\nPress: [a] for another location [g] to refresh [q] to quit") ;; align buffer to top (goto-char (point-min))) - ;; Enable wttrin-mode (sets up keybindings, read-only, font, etc.) - ;; Must be called before setting buffer-local variables - (wttrin-mode) - ;; Set location after mode initialization (mode calls kill-all-local-variables) (setq-local wttrin--current-location location-name) ;; Auto-generate debug diagnostics if debug mode is enabled (when (featurep 'wttrin-debug) - (wttrin--debug-mode-line-info)) - - ;; Clear any lingering messages from url-retrieve - (message nil)))) + (wttrin--debug-mode-line-info))))) (defun wttrin-query (location-name) "Asynchronously query weather of LOCATION-NAME, display result when ready." |