Skip to content

mocha: non-existent fonts in OS report as pending #226

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

Merged
devongovett merged 2 commits into from
May 20, 2022
Merged

mocha: non-existent fonts in OS report as pending #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28c01be
mocha: non-existent fonts report as pending
liborm85 May 1, 2020
e823cda
Merge branch 'master' into test-skip
devongovett May 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/subset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('font subsetting', function () {
});

it('should re-encode variation glyphs', function (done) {
if (!fs.existsSync('/Library/Fonts/Skia.ttf')) return done();
if (!fs.existsSync('/Library/Fonts/Skia.ttf')) return this.skip();
Copy link
Contributor

@Pomax Pomax May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

surely this should be an error, to be thrown instead, through an assertion after openSync that reports "font not found"? Skipping tests because the font files don't exist effectively introduces potential bugs as a side effect of (accidentally) removing a font

Copy link
Contributor Author

@liborm85 liborm85 May 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No, this PR is just addresses to solve potential bugs about which you writes. These are fonts that are not available in the repository for legal reasons. They are only in filesystem on OS, especially on MacOS.
If someone does not have these fonts, tests cannot fail with error message "font not found", they would never pass the tests and this is wrong behavior. Previously without existence of a file, tests passed "silently". Now is skipped it and is reported to user. This is the expected correct behavior in mocha tests.


let font = fontkit.openSync('/Library/Fonts/Skia.ttf', 'Bold');
let subset = font.createSubset();
Expand Down
12 changes: 10 additions & 2 deletions test/variations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@ import fs from 'fs';

describe('variations', function () {
describe('Skia', function () {
if (!fs.existsSync('/Library/Fonts/Skia.ttf')) { return; }
let font = fontkit.openSync('/Library/Fonts/Skia.ttf');
let font;
if (fs.existsSync('/Library/Fonts/Skia.ttf')) {
font = fontkit.openSync('/Library/Fonts/Skia.ttf');
}

beforeEach(function () {
if (!font) {
this.skip();
}
});

it('should get available variation axes', function () {
let axes = font.variationAxes;
Expand Down