Test output is not in gnu standard diagnostic format

[dabrahams]

Description

Example:

✘ Test nfaToDfa() recorded an issue at parsingTests.swift:107:7: Expectation failed: (n.recognizes(input) → false) == (r → true)
✘ Test nfaToDfa() recorded an issue at parsingTests.swift:108:7: Expectation failed: (d.recognizes(input) → false) == (r → true)

Now I have to write a custom parser to recognize these messages, when if they followed the standard, my IDE would "just work". The standard is here.

The messages also don't output full paths from the directory where swift test was invoked, which means I have to add some really hacky code to crawl through the sources and find the right file…maybe. I'm not even sure how to do that.
This is a regression from XCTest output and makes me want to go back!

See also swiftlang/swift-package-manager#6862

[grynspan]

Swift Testing is not a GCC-family compiler.

The human-readable (well… hopefully human-readable) output written to stderr is in an arbitrary format and is not meant to be parsed. Take a look at the machine-readable JSON output provided via --event-stream-output-path.

[grynspan]

The messages also don't output full paths from the directory where swift test was invoked,

So you're aware, Swift will not report full source file paths in the future, not even via XCTest. See SE-0285.

[dabrahams]

I'm aware. I've requested that there be a flag for debug builds that will provide full paths. also swiftlang/swift-package-manager#6862. It's just a matter of how interoperable you want to be with existing tools to integrate and how easy it will be for users to track down problems.

[dabrahams]

Meanwhile, here is the hack for Emacs. Unfortunately, every other tool will need its own hack.

[grynspan]

I'm not able to see that commit—looks like a permissions issue?

Our human-readable output is never going to be fully machine-readable, but we provide XML and JSON output that is (and the JSON output does contain full paths, although they have underscored keys and so are not officially supported.) So without being able to see your Emacs code, I might suggest teaching it how to read the JSON instead of stderr. :)

[dabrahams]

Sorry about the perms; code inline below.

I might suggest teaching it how to read the JSON instead of stderr. :)

The problem with that is that a) I have to invoke swift some special way to get the JSON, which is inconvenient, and b) I don't want to look at the JSON output in my compilation buffer. The barrier to entry for getting a system to actually use JSON output is quite high: you need to parse it, you need to have a system for turning it back into something human-readable, and you need a system for pointing it back at the source. And if you're invoking swift in a terminal, you're out of luck; you won't get good/complete information. If you want to solve the terminal problem, I guess you want to build a filter that you can pipe Swift through and turn the JSON into human-readable output, and build an alias/wrapper for swift that applies the filter. Then you might as well make that output Gnu-compliant, and nobody needs to mess with the JSON.

For years I was able to just M-x compile RET swift test RET in emacs and everything would work. Then someone decided to remove all the path information from #file and #line and I had to start working around that problem for development purposes. Now we have tools like swift-testing that don't want to let me work around the problem at all, And I presume JSON isn't going to help with that because swift-testing is just a library that will want to use plain #file and #line itself; the full path information won't be available—am I wrong about that?

  (add-to-list 'compilation-error-regexp-alist-alist
               `(swift-testing
                 ,(rx bol
                      (literal "✘ Test ") (+ (not space)) (literal " recorded an issue at ")
                      (group (+ (not (in "\n:")))) ; 1: file
                      ":"
                      (group (+ digit)) ; 2: line
                      ":"
                      (group (+ digit)) ; 3: column
                      )
                 1 2 3))

  (add-to-list 'compilation-error-regexp-alist 'swift-testing)

;; Enhanced file finding for compilation-mode
(require 'compile)
(require 'cl-lib)

(defvar compilation-smart-find-max-depth 5
  "Maximum depth for recursive file search.")

(defun compilation-smart-find-git-files ()
  "Get list of files tracked by git in current repository."
  (when (executable-find "git")
    (with-temp-buffer
      (when (zerop (call-process "git" nil t nil "ls-files"))
        (split-string (buffer-string) "\n" t)))))

(defun compilation-smart-find-recursive (filename &optional depth)
  "Find file recursively from current directory up to max depth."
  (let ((depth (or depth 0))
        (result nil))
    (when (< depth compilation-smart-find-max-depth)
      (dolist (file (directory-files-recursively
                     default-directory
                     (regexp-quote filename)))
        (when (and (file-exists-p file)
                  (not (file-directory-p file)))
          (push file result))))
    result))

(defun compilation-smart-find-file (filename)
  "Smart file finder for compilation-mode.
First tries git-ls-files, then falls back to recursive search."
  (let ((git-files (compilation-smart-find-git-files))
        matches)
    (cond
     ;; First try exact path
     ((file-exists-p filename)
      (list filename))

     ;; Then try git-ls-files
     (git-files
      (setq matches
            (cl-remove-if-not
             (lambda (f) (string-match-p (regexp-quote filename) f))
             git-files))
      (when matches
        matches))

     ;; Finally try recursive search
     (t
      (compilation-smart-find-recursive filename)))))

(defun compilation-smart-find-file-choose (matches filename)
  "Choose from multiple matching files."
  (cond
   ((null matches) nil)
   ((= (length matches) 1) (car matches))
   (t
    (completing-read
     (format "Multiple matches for '%s', choose one: " filename)
     matches nil t))))

(defun compilation-find-file-smart (orig-fun marker filename directory &rest args)
  "Advice around `compilation-find-file' to enhance file finding."
  (let ((original-result (apply orig-fun marker filename directory args)))
    (if (and (null original-result)
             (not (file-name-absolute-p filename)))
        (let* ((matches (compilation-smart-find-file filename))
               (chosen-file (compilation-smart-find-file-choose matches filename)))
          (if chosen-file
              (find-file-noselect chosen-file)
            original-result))
      original-result)))

;; Install the advice
(advice-add 'compilation-find-file :around #'compilation-find-file-smart)

;; Optional: Add a command to remove the advice if needed
(defun compilation-smart-find-disable ()
  "Remove smart file finding advice from compilation-mode."
  (interactive)
  (advice-remove 'compilation-find-file #'compilation-find-file-smart))

(provide 'compilation-smart-find)

[grynspan]

The problem with that is that a) I have to invoke swift some special way to get the JSON, which is inconvenient, and b) I don't want to look at the JSON output in my compilation buffer.

I don't speak Emacs' dialect of Lisp well enough to be sure how you're actually invoking swift test or Swift Testing here, but if you're calling swift test, presumably you can pass a command-line argument? If so, it's then just a matter of passing --event-stream-output-path /tmp/whatever.json and consuming the file afterwards. The JSON won't be written to stdout or stderr, so it presumably won't poison your buffer.

The barrier to entry for getting a system to actually use JSON output is quite high: you need to parse it, you need to have a system for turning it back into something human-readable, and you need a system for pointing it back at the source.

We've managed to make it work quite well with the Swift VS Code plugin, so perhaps the barrier isn't as high as you're guessing? It's worth investigating, I think.

And I presume JSON isn't going to help with that because swift-testing is just a library that will want to use plain #file and #line itself; the full path information won't be available—am I wrong about that?

Swift Testing's JSON output includes the values of #line, #column, #fileID, and (at least currently) #filePath for all relevant records. The schema is laid out here (apologies for the half-baked formatting). Although file paths aren't listed, they are present next to "fileID" under the key "_filePath". The underscore indicates we don't formally support this key, but it's likely to stick around for a while out of sheer usefulness.

[dabrahams]

I don't speak Emacs' dialect of Lisp well enough to be sure how you're actually invoking swift test or Swift Testing here,

It's not in this code; it's a simple emacs command that I make from inside the editor: cmd-X compile return swift test return (and I have cmd-X compile bound to a key), and I have a key to repeat the last compile command. Most of my development is editing punctuated by hitting that key.

Swift Testing's JSON output includes the values of #line, #column, #fileID, and (at least currently) #filePath for all relevant records.

In that case, you must be encoding the #filePath in the binary and I really don't see the point in omitting the path from the invocation directory to the file from the human-readable output.

We've managed to make it work quite well with the Swift VS Code plugin, so perhaps the barrier isn't as high as you're guessing? It's worth investigating, I think.

The thing is, VSCode has all the infrastructure for doing that; it has a first-class “problems” pane that you can plug into and whatnot. Emacs isn't set up that way; I'd have to invent a whole new subsystem just to handle Swift, and then that subsystem wouldn't work with the hundreds of other tools that Emacs already works with OOTB using compilation mode, or with anyone else's 3rd party customization for compilation mode.

Love it or hate it, one good property of Emacs' system is that for the vast majority of tools that report issues, you don't need to know how to write a plugin, or write any code at all. The tools that use the gnu standard format just work. The hundreds of others already in emacs' list of output scraping regexes just work. And if you're not golden at that point, you can usually just write an appropriate regex yourself (that's what the first stanza of my elisp code does)… provided that the tools cooperate and attach at least a relative path to the filename. Stripping the path is hostile to that system.

The underscore indicates we don't formally support this key, but it's likely to stick around for a while out of sheer usefulness.

Sounds like another reason for me to keep scraping the human-readable output.

[grynspan]

It's not in this code; it's a simple emacs command that I make from inside the editor:

Understood.

Love it or hate it, one good property of Emacs' system is that for the vast majority of tools that report issues, you don't need to know how to write a plugin, or write any code at all. The tools that use the gnu standard format just work.

Just to spitball here… I've toyed with the idea of having a ~/.swift-testing config file. Hypothetically, if we added something like "showFullPaths" = true to such a config file (that we would read at the start of a test run), would that be useful to you here?

Sounds like another reason for me to keep scraping the human-readable output.

I can't stop you from doing so, I can only advise against it. 🙂

[dabrahams]

if we added something like "showFullPaths" = true to such a config file (that we would read at the start of a test run), would that be useful to you here?

Yes please! showRelativePaths would be even more useful, as it would strip redundant info from the output.

I can only advise against it.

Given what you now know about my environment and the fact that I already have it working, you would really advise that I do something else? If so, why?

[grynspan]

if we added something like "showFullPaths" = true to such a config file (that we would read at the start of a test run), would that be useful to you here?

Yes please! showRelativePaths would be even more useful, as it would strip redundant info from the output.

I'll keep this request in mind for when we stabilize the format of our configuration data.

I can only advise against it.

Given what you now know about my environment and the fact that I already have it working, you would really advise that I do something else? If so, why?

The human-readable output is subject to change over time and we make no guarantees that it will remain compatible with scripting you build around it.

[dabrahams]

The human-readable output is subject to change over time and we make no guarantees that it will remain compatible with scripting you build around it.

I understand that of course. Apparently the inclusion of full paths in the JSON is equally subject to change, and I already have this functionality, and the effort to use the other functionality and still produce a good user experience is high. I just don't see how it makes any sense to change what I'm doing right now.

[dabrahams]

BTW, speaking of changing the human-readable text, what does “recorded an issue” mean? Does it write some auxiliary file? Why not a much more straightforward message that a test "failed," which most people would understand right away? “An issue” doesn't necessarily mean a test failure.

[grynspan]

I just don't see how it makes any sense to change what I'm doing right now.

As I wrote, I can't stop you from doing what you're doing, I can only advise against it. 🙂

BTW, speaking of changing the human-readable text, what does “recorded an issue” mean? Does it write some auxiliary file? Why not a much more straightforward message that a test "failed," which most people would understand right away? “An issue” doesn't necessarily mean a test failure.

An issue is an instance of Issue. It is recorded in the test log (which can be any of stderr, Xcode's Test Report, JSON, XUnit/JUnit XML, or whatever VS Code calls their testing UI.)

And, as you note, an issue doesn't necessarily mean a test failure. Right now, issues may be known (not failures) or unknown (failures) but we may introduce the concept of issue severity in the future, in which case an issue may occur and be recorded and only result in a warning or advisory (exact details TBD.)

[dabrahams]

As I wrote, I can't stop you from doing what you're doing, I can only advise against it. 🙂

Yeah, I heard you. I can't understand why, given everything you know about my circumstances and the systems I have to integrate with, you'd still be advising me against doing it in the short term. It makes me think maybe I'm missing something.

IMO it's really important to tell users explicitly when something is a failure, that it's a failure—especially if you're going to have non-failure issues. I strongly recommend making the failure text explicit about that.