Skip to content

Add unit test and github actions configuration #101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 22, 2022
Merged

Add unit test and github actions configuration #101

merged 5 commits into from
Oct 22, 2022

Conversation

syohex
Copy link
Collaborator

@syohex syohex commented Oct 22, 2022

Changing

  • Add unit tests
  • Add github actions configuration

This patch took #98 changes for adding unit test

CC: @liuyinz

@syohex syohex requested a review from wasamasa October 22, 2022 08:29
@syohex syohex marked this pull request as ready for review October 22, 2022 08:31
@eki3z
Copy link
Contributor

eki3z commented Oct 22, 2022

Tests good, please merge it.

@@ -0,0 +1,117 @@
;;;; yaml-mode-test.el --- Tests for yaml-mode -*- lexical-binding: t; -*-

;; Copyright (C) 2010-2022 Yoshiki Kurihara
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect, you wrote the file, so you have the copyright and authorship.


(require 'yaml-mode)
(require 'ert)
(require 'cl-lib)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import.

;; for version < 25
(defconst yaml-test-font-lock-function
(if (fboundp 'font-lock-ensure)
#'font-lock-ensure #'font-lock-fontify-buffer))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either put the whole if expression on one line or every branch on a line.

(if (fboundp 'font-lock-ensure)
#'font-lock-ensure #'font-lock-fontify-buffer))

(defmacro yaml-test-string-mode (mode string &rest body)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a premature abstraction. Since only yaml-mode is tested, both macros can be combined into one:

  • Change the (funcall ,mode) line to (yaml-mode)
  • Rename this macro to yaml-test-string
  • Delete the other macro

yaml-mode.el Outdated
@@ -178,7 +178,7 @@ that key is pressed to begin a block literal."
"y" "Y" "yes" "Yes" "YES" "n" "N" "no" "No" "NO"
"true" "True" "TRUE" "false" "False" "FALSE"
"on" "On" "ON" "off" "Off" "OFF") t)
" *$")
"\\b")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested some more meanwhile and it does match True#123 which it shouldn't. So \\_> is better.

@wasamasa
Copy link
Collaborator

Generally speaking, I'm in for automating these checks. What do you think of using an existing solution for testing the font-lock bits, like using assess-face-at= from assess or faceup?


- uses: actions/checkout@v3
- name: Install dependencies
run: sudo apt install pandoc aspell
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pandoc and aspell?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is wrong copy and paste, I'll fix later

@@ -1,6 +1,6 @@
;;;; yaml-mode-test.el --- Tests for yaml-mode -*- lexical-binding: t; -*-

;; Copyright (C) 2010-2022 Yoshiki Kurihara
;; Copyright (C) 2022 - Shohei YOSHIDA

;; Author: Yoshiki Kurihara <clouder@gmail.com>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright and author are mismatched

@syohex
Copy link
Collaborator Author

syohex commented Oct 22, 2022

@wasamasa I've fixed your comments

@syohex
Copy link
Collaborator Author

syohex commented Oct 22, 2022

What do you think of using an existing solution for testing the font-lock bits, like using assess-face-at= from assess or faceup?

I have never use such tools and I have never seen projects that use those frameworks, sorry.

@syohex syohex merged commit 141b85f into master Oct 22, 2022
@syohex syohex deleted the support-ci branch October 22, 2022 09:20
@syohex
Copy link
Collaborator Author

syohex commented Oct 22, 2022

Thank you for reviewing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants