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.

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

ufolint should run tests on all UFOs and display errors at end of run instead of stopping after the first erroneous UFO #27

Open
madig opened this issue Oct 24, 2018 · 5 comments

Comments

@madig
Copy link
Contributor

madig commented Oct 24, 2018

Errors can then be fixed in one go instead of requiring re-runs after each UFO has been fixed.

@chrissimpkins
Copy link
Member

chrissimpkins commented Oct 24, 2018

It should be doing this for many of the tests. There were a number of tests that I defined as "early fail" for things like a bad directory path, missing *.ufo extension, cannot determine UFO spec version in the source.

The block of tests is here:

https://github.com/source-foundry/ufolint/blob/master/lib/ufolint/controllers/runner.py#L42-L104

After that block, we should be capturing all failures and reporting everything at the end. So, all XML validation failures, failures on *.glif files, etc. should all be reported at the end.

Do you have examples where this does not occur? If so, we may need a better set of test files. Agree with this suggestion and definitely willing to take PR/work on this myself if there are specific instances where we can identify that this is not working as intended.

@chrissimpkins
Copy link
Member

chrissimpkins commented Oct 24, 2018

Sorry I may have misunderstood here. On re-read it sounds like you want to run all tests across all requested UFO sources irrespective of errors and fail late after all tests x UFO directories has been performed? As it stands we are failing early on a UFO directory when an error is detected in a given UFO. Is that correct?

@chrissimpkins
Copy link
Member

And if #27 (comment) is correct, it is possible to sidestep the issue by testing in parallel. Not what you are asking for, but it will work until this is addressed. Most CI services allow you to parallelize your tests in a similar fashion. See the Travis CI setup on README.

@madig
Copy link
Contributor Author

madig commented Oct 24, 2018

If I have 2 broken UFOs like in Temp.zip and run the command ufolint test.ufo test2.ufo then it will display the errors of the first UFO but not proceed to test the second one. I'd like ufolint to run on all UFOs and display a summary with errors for all UFOs separately. I could run ufolint in parallel or separately but that would make the output confusing because you'd have to scroll through the output to spot errors instead of just looking at the bottom.

@chrissimpkins
Copy link
Member

chrissimpkins commented Oct 24, 2018

Sorry, by in parallel I meant in parallel as in through individual jobs during the CI testing. The Travis setup on the README tests each master UFO directory in a separate job so you are in effect running ufolint simultaneously on all masters with separate stdout/err for each ufolint check on a given master. You map the job : master in a 1:1 fashion. Then, click on the job for the master that you are interested in and you see the ufolint report for that master only. You can parallelize that to as many parallel jobs as the Travis account supports. For OS work, I believe that it is four. For paid accounts, the sky is probably the limit depending upon how much you are willing to pay. If any of the individual jobs fail, the entire build fails and Travis flags as a failed build. Then you can view the master(s) that failed for the issue.

That is an aside as a workaround though. You are right. We should be able to support this. I need to dig through the code to see how involved it would be. I haven't touched the source here for a bit.

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

No branches or pull requests

2 participants