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

Travis test environment not picking up ICU #165

Closed
alerque opened this issue Aug 15, 2015 · 5 comments · Fixed by #172
Closed

Travis test environment not picking up ICU #165

alerque opened this issue Aug 15, 2015 · 5 comments · Fixed by #172
Assignees
Labels
ci Continuous integration pipelines

Comments

@alerque
Copy link
Member

alerque commented Aug 15, 2015

Per discussion subsequent to #160, configure is not picking up ICU support in the Ubuntu test machine. The correct pkg-config file is probably going to have to be patched in after install.

So this is why: pkg-config files weren't shipped by Debian or Ubuntu

Related to #146 and possibly one more reason we can't have nice things. Yet.

@alerque alerque self-assigned this Aug 15, 2015
@alerque
Copy link
Member Author

alerque commented Aug 15, 2015

Just to verify @simoncozens on my system as of these test updates I'm down
to three failed tests that are explained by various outstanding issues:

Can you confirm the same on your system?

On the other hand Travis is reporting this set:

  • tests/arabic-scripts.sil
  • tests/balanced.sil
  • tests/bug-132.sil
  • tests/bug-76.sil
  • tests/bug-79.sil
  • tests/mini-arabic.sil
  • tests/process-commands.sil
  • tests/space-after-command.sil
  • tests/vertical.sil

And I believe the discrepancy between these failure sets is explained
entirely by this ICU issue. As soon as Travis is fixed to find ICU support
all but the last three should start passing except in the instance where
ICU is explicitly not included, in which case a limited set of regression
tests should be run so as not to run into these differences but still make
sure basic parsing works.

Does that scan by you?

@simoncozens
Copy link
Member

Here's what I see failing on my system at the moment:

  • tests/balanced.sil
  • tests/bug-75_darwin.sil
  • tests/process-commands.sil
  • tests/space-after-command.sil

bug-75_darwin has never worked on my system. I have no idea why balanced is failing. And yes, the other two are known-bad.

I am just about to add a known-bad test category to the test harness, because I really want to get back to all tests passing - it's hard to do development if you can't actually trust the regression test results and have to mentally keep track of what should be failing and what shouldn't.

Yes, the remaining discrepancy is down to ICU. I want to improve the tokenising a bit, but at the same time if I end up re-implementing the whole of ICU's breaking support in Lua, then there's not a huge amount of point ever building with ICU... My thinking is that the idea behind ICU support would be that special-case people using specially exotic languages would want to have it compiled in, but most people wouldn't need to bother. I need to work out where the cut-off for "exotic" is. I think Arabic needs to work. I think I have just fixed tokenising for Kannada. So I will get Arabic tokenising working and then see what else shakes out of the tests.

@simoncozens
Copy link
Member

Just to further clarify and for the record, if I manually disable ICU support on my system the following further tests fail:

  • tests/arabic-scripts.sil
  • tests/bug-132.sil
  • tests/mini-arabic.sil

I'm going to do knownbad first and then work on getting the tokenising working for these tests.

@alerque
Copy link
Member Author

alerque commented Aug 15, 2015

If you want I'll work on a knownbad flag of some kind. But it might not be done until Monday as I have some sermon stuff to work on today. I guess if you don't do it by then I'll tackle it then.

In other news tests/balanced.sil actually shouldn't be passing (see #118) and we can probably add it to the known bad list for now. It does seem like there might still be another metric related problem turning up there but I can't put my finger on it.

@simoncozens simoncozens added the ci Continuous integration pipelines label Aug 16, 2015
@alerque
Copy link
Member Author

alerque commented Aug 17, 2015

So one option is to use the assorted Debian packaging tools to download the icu source package, patch the Debian recipe to include the *.pc files, recompile the package and install it. I got that working but the ICU build takes a (short) week to build which really doesn't make sense to keep repeating inside Travis.

Instead I've tared up the *.pc files that process generated and stashed them in a (base 64 encoded) gist that can be extracted to the system root. Bypassing the package manager like this is a slimy thing to do to a real system but since we're in a VM that gets blown away every time anyway and we're just trying to bootstrap the process so we can test sile I don't really care about the host being clean.

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

Successfully merging a pull request may close this issue.

2 participants