Skip to content

Commit

Permalink
k8s-pod-line: ok-p: Conform to -let edebug spec
Browse files Browse the repository at this point in the history
The top-level `-let` specification in `kubernetes-pod-line-ok-p` does
not conform to `-let`'s edebug spec, which reads as follows:

```emacs-lisp
[&or (&rest [&or (sexp form) sexp]) (vector [&rest [sexp form]])]
```

Specifically, it repeats `pod` to provide a length-3 vector rather
than the expected length-2 vector.

This causes `undercover` to fail to parse `kubernetes-pod-line.el` for
coverage, with the following error:
```
UNDERCOVER: Error while loading /Users/jjin/dev/kubernetes-el/kubernetes-pod-line.el for coverage:
UNDERCOVER: Invalid read syntax: "Expected one of", (&rest [&or (sexp form) sexp]), (vector [&rest [sexp form]])
UNDERCOVER: The problem may be due to edebug failing to parse the file.
UNDERCOVER: You can try to narrow down the problem using the following steps:
UNDERCOVER: 1. Open "/Users/jjin/dev/kubernetes-el/kubernetes-pod-line.el" in an Emacs buffer;
UNDERCOVER: 2. Run M-: ‘(require 'edebug)’;
UNDERCOVER: 3. Run M-x ‘edebug-all-defs’;
UNDERCOVER: 4. Run M-x ‘toggle-debug-on-error’.
UNDERCOVER: 5. Run M-x ‘eval-buffer’;
UNDERCOVER: 6. In the *Backtrace* buffer, find a numeric position,
UNDERCOVER:    then M-x ‘goto-char’ to it.
```

However, this non-compliance with the edebug turns out to not be
fatal, nor has it ever been fatal, due to the fact that `-let`, when
the `VARLIST` argument is a vector, explicitly pulls out the 0th and
1st elements of that vector. This implementation detail presumably
making our repeated element here non-fatal at runtime, but causes
edebug to complain -- both in `undercover`, and if you were to run
`edebug-defun` on `kubernetes-pod-line-ok-p` definition at point.

As such, here, we remove the index-2 element in the vector, helping
`undercover` parse this file properly and hopefully providing a nice
boost to test coverage.
  • Loading branch information
jinnovation committed Jun 4, 2021
1 parent b887d96 commit f12cfa5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
4 changes: 3 additions & 1 deletion kubernetes-pod-line.el
Expand Up @@ -7,10 +7,12 @@
(require 'kubernetes-utils)

(defun kubernetes-pod-line-ok-p (pod)
(-let [(&alist 'status (&alist 'containerStatuses containers 'phase phase)) pod pod]
"Determine if POD should be displayed with a warning or not."
(-let [(&alist 'status (&alist 'containerStatuses containers 'phase phase)) pod]
(unless (seq-empty-p containers)
(-let* (([(&alist 'state pod-state)] containers)
(pod-state (or (alist-get 'reason (alist-get 'waiting pod-state)) phase)))
;; (msesage pod-state)
(member (downcase pod-state) '("running" "containercreating" "terminated"
"succeeded"))))))

Expand Down
17 changes: 17 additions & 0 deletions test/kubernetes-pod-line-test.el
@@ -0,0 +1,17 @@
;;; kubernetes-pod-line-test.el --- Test rendering of the pods lines -*- lexical-binding: t; -*-
;;; Commentary:
;;; Code:

(require 'kubernetes-pod-line)

(declare-function test-helper-json-resource "test-helper.el")

(defconst sample-get-pods-response (test-helper-json-resource "get-pods-response.json"))

(ert-deftest kubernetes-pod-line-test--pod-line-ok-p ()
(let* ((state `((pods . ,sample-get-pods-response)))
(pods (kubernetes-state-pods state))
(pod (aref (alist-get 'items pods) 0)))
(should-not (null (kubernetes-pod-line-ok-p pod)))))

;;; kubernetes-pod-line-test.el ends here

0 comments on commit f12cfa5

Please sign in to comment.