Skip to content

Commit

Permalink
Fixes broken test behaviors on old and new ICUs
Browse files Browse the repository at this point in the history
With the following changes, we should be well positioned to tackle
ICU 67.1

- ICU4C correctly implements language fallbacks starting version 67.1,
  added two flavors of the same test, one for ICU versions 67 and
  onwards, and another for older versions.  Feature:
  `icu_version_67_plus`.  This happens in issue google#59.
- ICU4C version 63 has wrong result for a certain test, possibly due to
  the CLDR version on my machine, so test failed.  Turned that test off
  for versions less than 64.  This happens on my machine only, when the
  system ICU is used.
  • Loading branch information
filmil committed May 8, 2020
1 parent ed1d068 commit 4b8cf84
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 11 deletions.
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ endif
# NOTE: This version number is completely independent of the crate version.
USED_BUILDENV_VERSION ?= 0.0.4

CARGO_FEATURE_VERSION :=

ICU_LIBDIR := $(shell icu-config --libdir)
test:
env PKG_CONFIG_PATH="${HOME}/local/lib/pkgconfig" \
@env PKG_CONFIG_PATH="${HOME}/local/lib/pkgconfig" \
LD_LIBRARY_PATH="${ICU_LIBDIR}" \
cargo test
echo "ICU version detected: $(shell icu-config --version)" && \
cargo test
.PHONY: test

# Run a test inside a Docker container. The --volume mounts attach local dirs
Expand Down
17 changes: 9 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

| Item | Description |
| ---- | ----------- |
| ICU 64/65/66 | [![Build Status `master`](https://travis-ci.org/google/rust_icu.svg?branch=master)](https://travis-ci.org/google/rust_icu) |
| ICU 64..67 | [![Build Status `master`](https://travis-ci.org/google/rust_icu.svg?branch=master)](https://travis-ci.org/google/rust_icu) |
| Source | https://github.com/google/rust_icu |
| README | https://github.com/google/rust_icu/blob/master/README.md |
| Coverage | [View report](/coverage/report.md)
| Docs | https://github.com/google/rust_icu/blob/master/docs/README.md |
| Docs | https://docs.rs/crate/rust_icu |

This is a library of low level native rust language bindings for the
International Components for Unicode (ICU) library for C (a.k.a. ICU4C).
Expand Down Expand Up @@ -98,12 +98,13 @@ headers of columns 2 and onwards are features set combos. The coverage
reflects the feature set and version points that we needed up to this point.
The version semver in each cell denotes the version point that was tested.

| ICU version | `default` | `renaming` | `renaming`, `icu_version_in_env`|
| ----------- | ------------------- | ---------------------- | ----- |
| 63.x | ??? | ??? | ??? |
| 64.2 | 0.1.3 | ??? | ??? |
| 65.1 | 0.1.3 | 0.1.3 | 0.1.3 |
| 66.0.1 | 0.1.3 | ??? | ??? |
| ICU version | `default` | `renaming` | `renaming`, `icu_version_in_env` |
| ----------- | ----------| ---------- | --------------------------------- |
| 63.x | ??? | - | - |
| 64.2 | 0.1.3+ | - | - |
| 65.1 | 0.1.3+ | 0.1.3+ | 0.1.3+ |
| 66.0.1 | 0.1.3+ | - | - |
| 67.1 | 0.1.4 | - | - |

> API versions that differ in the minor version number only should be
> compatible; but since it is time consuming to test all versions and
Expand Down
8 changes: 8 additions & 0 deletions rust_icu_sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ impl ICUConfig {
.with_context(|| format!("could not parse version number: {}", version))?;
Ok(last.to_string())
}

fn version_major_int() -> Result<i32> {
let version_str = ICUConfig::version_major()?;
Ok(version_str.parse().unwrap())
}
}

/// Returns true if the ICU library was compiled with renaming enabled.
Expand Down Expand Up @@ -321,6 +326,9 @@ fn copy_features() -> Result<()> {
if let Some(_) = env::var_os("CARGO_FEATURE_ICU_VERSION_IN_ENV") {
println!("cargo:rustc-cfg=features=\"icu_version_in_env\"");
}
if ICUConfig::version_major_int()? >= 67 {
println!("cargo:rustc-cfg=features=\"icu_version_67_plus\"");
}
Ok(())
}

Expand Down
7 changes: 7 additions & 0 deletions rust_icu_uloc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ authors = ["Google Inc."]
edition = "2018"
license = "Apache-2.0"
name = "rust_icu_uloc"
build = "build.rs"
readme = "README.md"
repository = "https://github.com/google/rust_icu"
version = "0.1.3"
Expand Down Expand Up @@ -54,6 +55,12 @@ icu_version_in_env = [
"rust_icu_uenum/icu_version_in_env",
"rust_icu_ustring/icu_version_in_env",
]
icu_version_64_plus = []
icu_version_67_plus = []

[build-dependencies]
anyhow = "1.0"
bindgen = "0.53.2"

[badges]
maintenance = { status = "actively-developed" }
Expand Down
109 changes: 109 additions & 0 deletions rust_icu_uloc/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
#![feature(try_trait)]
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// See LICENSE for licensing information.
//
// This build.rs script tries to generate low-level rust bindings for the current ICU library.
// Please refer to README.md for instructions on how to build the library for
// your use.

use {
anyhow::{Context, Result},
std::process,
};

/// A `Command` that also knows its name.
struct Command {
name: String,
rep: process::Command,
}

impl Command {
/// Creates a new command to run, with the executable `name`.
pub fn new(name: &'static str) -> Self {
let rep = process::Command::new(&name);
let name = String::from(name);
Command { name, rep }
}

/// Runs this command with `args` as arguments.
pub fn run(&mut self, args: &[&str]) -> Result<String> {
self.rep.args(args);
let stdout = self.stdout()?;
Ok(String::from(&stdout).trim().to_string())
}

// Captures the stdout of the command.
fn stdout(&mut self) -> Result<String> {
let output = self
.rep
.output()
.with_context(|| format!("could not execute command: {}", self.name))?;
let result = String::from_utf8(output.stdout)
.with_context(|| format!("could not convert output to UTF8"))?;
Ok(result.trim().to_string())
}
}

/// A command representing an auto-configuration detector. Use `ICUConfig::new()` to create.
struct ICUConfig {
rep: Command,
}

impl ICUConfig {
/// Creates a new ICUConfig.
fn new() -> Self {
ICUConfig {
rep: Command::new("pkg-config"),
}
}
/// Obtains the major-minor version number for the library. Returns a string like `64.2`.
fn version(&mut self) -> Result<String> {
self.rep
.run(&["--modversion", "icu-i18n"])
.with_context(|| format!("while getting ICU version; is icu-config in $PATH?"))
}

/// Returns the config major number. For example, will return "64" for
/// version "64.2"
fn version_major() -> Result<String> {
let version = ICUConfig::new().version()?;
let components = version.split(".");
let last = components
.take(1)
.last()
.with_context(|| format!("could not parse version number: {}", version))?;
Ok(last.to_string())
}

fn version_major_int() -> Result<i32> {
let version_str = ICUConfig::version_major()?;
Ok(version_str.parse().unwrap())
}
}

fn main() -> Result<()> {
std::env::set_var("RUST_BACKTRACE", "full");
let icu_major_version = ICUConfig::version_major_int()?;
println!("icu-major-version: {}", icu_major_version);
if icu_major_version >= 64 {
println!("cargo:rustc-cfg=features=\"icu_version_64_plus\"");
}
if icu_major_version >= 67 {
println!("cargo:rustc-cfg=features=\"icu_version_67_plus\"");
}
println!("done");
Ok(())
}
33 changes: 32 additions & 1 deletion rust_icu_uloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,10 +585,13 @@ mod tests {
Ok(())
}

// This test yields a different result in ICU versions prior to 64:
// "zh-Latn@collation=pinyin".
#[cfg(features = "icu_version_64_plus")]
#[test]
fn test_variant() -> Result<(), Error> {
let loc = ULoc::try_from("zh-Latn-pinyin")?;
assert_eq!(loc.variant(), Some("PINYIN".to_string()));
assert_eq!(loc.variant(), Some("PINYIN".to_string()), "locale was: {:?}", loc);
Ok(())
}

Expand Down Expand Up @@ -756,6 +759,8 @@ mod tests {
);
}

// This tests verifies buggy behavior which is fixed since ICU version 67.1
#[cfg(not(features = "icu_version_67_plus"))]
#[test]
fn test_accept_language_exact_match() {
let accept_list: Result<Vec<_>, _> = vec!["es_ES", "ar_EG", "fr_FR"]
Expand All @@ -774,12 +779,38 @@ mod tests {
assert_eq!(
actual,
(
// "es_MX" should be preferred as a fallback over exact match "ar_EG".
ULoc::try_from("ar_EG").ok(),
UAcceptResult::ULOC_ACCEPT_VALID
)
);
}

#[cfg(features = "icu_version_67_plus")]
#[test]
fn test_accept_language_exact_match() {
let accept_list: Result<Vec<_>, _> = vec!["es_ES", "ar_EG", "fr_FR"]
.into_iter()
.map(ULoc::try_from)
.collect();
let accept_list = accept_list.expect("make accept_list");

let available_locales: Result<Vec<_>, _> = vec!["de_DE", "en_US", "es_MX", "ar_EG"]
.into_iter()
.map(ULoc::try_from)
.collect();
let available_locales = available_locales.expect("make available_locales");

let actual = accept_language(accept_list, available_locales).expect("call accept_language");
assert_eq!(
actual,
(
ULoc::try_from("es_MX").ok(),
UAcceptResult::ULOC_ACCEPT_FALLBACK,
)
);
}

#[test]
fn test_accept_language_no_match() {
let accept_list: Result<Vec<_>, _> = vec!["es_ES", "ar_EG", "fr_FR"]
Expand Down

0 comments on commit 4b8cf84

Please sign in to comment.