Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expand test extensions and fix whitespace checks #1156

Merged
merged 1 commit into from
Apr 5, 2020
Merged

Expand test extensions and fix whitespace checks #1156

merged 1 commit into from
Apr 5, 2020

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented Mar 28, 2020 •
edited
Loading

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Bug fix

Fixes #1131 - expand test extensions, fix whitespace - newline sensitivity
Fixes #254 - test 'asset files'

What is the rationale for this request?
To fix / improve our functional test utilities.

What changes did you make? (Give an overview)

  • Run the diffing on all text files in our expected test sites.
  • Fix whitespace - newline sensitivity
    • As a result, to display whitespaces / newlines in the diff properly, the diff printing was also slightly improved - whitespace diffs are colored with bgGreen/Red and empty lines ( that are touched ) are shown as |-----empty-line-----| ( 50 characters long ):

Provide some example code that this change will affect:

Filtering of files to be diffed using isTextOrBinary package, followed by a manual blacklist for file types not recognised ( e.g. .woff/woff2 ) / other files we want to not diff ( .log )

if (isBinary(expectedFilePath) || TEST_BLACKLIST.ignores(expectedFilePath)) {
  continue;
}

Is there anything you'd like reviewers to focus on?
na

Testing instructions:

  • Make some changes to any text file in the expected test sites', the diff should be presented as is in the above image

Proposed commit message: (wrap lines at 72 characters)
Expand test extensions and fix whitespace checks

The diffing utilities used for functional tests only check for
differences in html files and the siteData json file.
Additionally, the diffs are whitespace and newline insensitive, which
can cause subtle, unintended changes to go unnoticed.

Let's expand the range of files to diff, using the isTextOrBinary
package as a first guard against diffing binary files.
Let's add additional constants that guard against binary files not
recognised by the package, or misrecognised as such.

Let's also change the diffing function used to be whitespace and
newline sensitive.
Since whitespaces and newlines now appear in the diffs, let's also
improve the diff printing utilities to account for it.

@ang-zeyu ang-zeyu changed the title Expand test extensions and fix whitespace checks [WIP] Expand test extensions and fix whitespace checks Mar 28, 2020
@ang-zeyu ang-zeyu changed the title [WIP] Expand test extensions and fix whitespace checks Expand test extensions and fix whitespace checks Mar 28, 2020
@le0tan le0tan added the pr.BugFix 🐛 Fixes correct a programming error/assumption label Mar 29, 2020
@ang-zeyu ang-zeyu force-pushed the expand-tests branch 2 times, most recently from f2a1638 to 4d6a78f Compare March 30, 2020 06:05
le0tan
le0tan approved these changes Apr 5, 2020
View reviewed changes
Copy link
Contributor

@le0tan le0tan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

test/functional/testUtil/diffChars.js Outdated
* @param {string} expected
* @param {string} actual
* @param {string} filePathName
* @returns {bool} if diff was found
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be boolean for JSDoc types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the review!

If there's no other issues i'll merge this in a while since it affects all PRs

@ang-zeyu
Expand test extensions and fix whitespace checks
68d8aa3 
The diffing utilities used for functional tests only check for
differences in html files and the siteData json file.
Additionally, the diffs are whitespace and newline insensitive, which
can cause subtle, unintended changes to go unnoticed.

Let's expand the range of files to diff, using the isTextOrBinary
package as a first guard against diffing binary files.
Let's add additional constants that guard against binary files not
recognised by the package, or misrecognised as such.

Let's also change the diffing function used to be whitespace and
newline sensitive.
Since whitespaces and newlines now appear in the diffs, let's also
improve the diff printing utilities to account for it.
@ang-zeyu ang-zeyu added this to the v2.13.2 milestone Apr 5, 2020
@ang-zeyu ang-zeyu merged commit 1519ad1 into MarkBind:master Apr 5, 2020
Tejas2805 added a commit to Tejas2805/markbind that referenced this pull request Apr 9, 2020
@Tejas2805
Merge branch 'master' of https://github.com/MarkBind/markbind
9792d68 
* 'master' of https://github.com/MarkBind/markbind: (41 commits)
  Document adding new site content in DG (MarkBind#1153)
  Add relative date feature (MarkBind#908)
  Use <br> to separate lines of code block (MarkBind#1176)
  Parse popovers for footnotes (MarkBind#1155)
  Resolve comments
  Expand test extensions and fix whitespace checks (MarkBind#1156)
  Address comments
  Upgrade js-beautify and provide option to turn it off (MarkBind#1116)
  Normalize inline puml line ending before hashing (MarkBind#1174)
  Update tests (MarkBind#1178)
  Remove fixed bugs from test\functional\test_site\bugs\index.md` (MarkBind#1148)
  Fix bug in Search for UG and DG (MarkBind#1147)
  Add inline puml support (MarkBind#1100)
  Fix hrefs for headings with angular brackets (MarkBind#1089)
  Update tests for 2.13.1 (MarkBind#1169)
  2.13.1
  Update vue-strap version to v2.0.1-markbind.39
  Fix fontawsome icons don't show underlines to indicate modal/tooltip (MarkBind#1133)
  2.13.0
  Update test files
  ...
marvinchin pushed a commit that referenced this pull request Apr 10, 2020
@ang-zeyu
Expand test extensions and fix whitespace checks (#1156)
7c3c160 
The diffing utilities used for functional tests only check for
differences in html files and the siteData json file.
Additionally, the diffs are whitespace and newline insensitive, which
can cause subtle, unintended changes to go unnoticed.

Let's expand the range of files to diff, using the isTextOrBinary
package as a first guard against diffing binary files.
Let's add additional constants that guard against binary files not
recognised by the package, or misrecognised as such.

Let's also change the diffing function used to be whitespace and
newline sensitive.
Since whitespaces and newlines now appear in the diffs, let's also
improve the diff printing utilities to account for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@le0tan le0tan le0tan approved these changes

@nbriannl nbriannl Awaiting requested review from nbriannl

Assignees
No one assigned
Labels
pr.BugFix 🐛 Fixes correct a programming error/assumption
Projects
None yet
Milestone
v2.14.0
2 participants
@ang-zeyu @le0tan