#1 Add CI tests
Opened 4 years ago by tagoh. Modified 4 years ago
rpms/ tagoh/dejavu-fonts master  into  rawhide

Exclude some fonts to pass testing
Akira TAGOH • 4 years ago  
Add CI tests
Akira TAGOH • 4 years ago  
file added
+2
@@ -0,0 +1,2 @@ 

+ /artifacts

+ *~

@@ -0,0 +1,69 @@ 

+ # Ansible role for tests using fontconfig

+ 

+ Put this role in your `tests.yml` playbook. The playbook will first install

+ package dependencies listed on playbook on your localhost, then it will proceed

+ to run testing.  You can redefine the following variables:

+ 

+  * **artifacts**: An artifacts directory on localhost to store logs

+  * **remote_artifacts**: The directory on the system under test where the logs

+    are stored. Note: if this variable is left undefined, it will default to

+    `/tmp/artifacts`

+  * **required_packages**: A list of prerequisite packages required by tests.

+    Please note that for Atomic Host, additional packages will be installed

+    using the `rpm-ostree` command which is affecting the test subject (it's

+    similar as rebuilding an rpm package to be tested) so this should be used

+    with caution on only when necessary.

+  * **path_prefix**: The directory on the system where fonts are installed.

+    please use one in coverage sub-parameter if having different path_prefix

+    per sub-packages.

+  * **package**: The package name to test. this is used to find out fontconfig

+    config file. please use one in families sub-parameter if having different

+    config files per sub-packages.

+  * **coverage**: A list of languages for language coverage tests.

+  * **families**: A list of family test cases.

+ 

+ ## Language coverage test parameters

+ 

+ Supporting two types of formats. one is a simple list of languages:

+ 

+     coverage:

+       - en

+       - fr

+ 

+ Another one is a dictionary that has a language as a key and values as parameters:

+ 

+     coverage:

+       en:

+         ...

+       fr:

+         ...

+ 

+ You can redefine the following variables for dictionary format:

+ 

+  * **exclude**: A list of font file names to exclude on this testing. this is

+    useful to avoid unexpected failures on iterating tests when a package has

+    multiple font files and has different coverages but you need to prevent

+    testing for few fonts which has different coverages to them.

+    Please note that the file name is relative to `path_prefix` parameter. also

+    good to consider using `include` if non-targeted files is more than targeted.

+  * **include**: A list of font file names to include on this testing. this is

+    useful to avoid unexpected failures on iterating tests when a pcakge has

+    multiple font files and has different coverages but you need to prevent

+    testing for few fonts which has different coverages to them.

+    Please note that the file name is relative to `path_prefix` parameter. also

+    good to consider using `exclude` if targeted files is more than non-targeted.

+  * **name**: The name to store logs. the test script is trying to make an unique

+    file names to store logs but not perfectly working in some cases. this is

+    optional parameter to make it unique by yourself.

+  * **path_prefix**: A list of directory names where fonts are installed on system.

+    this is optional parameter and tries to obtain the font paths from installed

+    packages by `required_packages` if not available.

+ 

+ ## Family test parameters

+ 

+  * **lang**: A language to test family name for.

+  * **alias**: An alias name to test.

+  * **family**: A family name to test, which is supposed to be assinged to the alias.

+  * **package**: The package name to test. this is used to find out fontconfig

+    config file. this is optional. if not specified here, global `package`

+    parameter will be used.

@@ -0,0 +1,162 @@ 

+ #! /bin/bash -efu

+ 

+ debug() {

+ 	if [ -n "$DEBUG" ]; then

+ 		echo "$*" >&2

+ 	fi

+ }

+ 

+ msg_usage() {

+ 	cat <<_EOF_

+ Run family test.

+ 

+ Usage:

+ $PROG <options>

+ 

+ Options:

+ -h, --help		Display this help and exit

+ -v, --verbose		Turn on debug

+ -l, --lang=LANG		Test LANG language coverage (default: en)

+ -f, --family=FILE	Set a family name supposed to be assigned for alias.

+ -g, --alias=STR		Set an alias name. (default: sans-serif)

+ -a, --artifactsdir=DIR	Set environment dir to store artifacts

+ -k, --package=NAME	Set a package name for fonts.

+ _EOF_

+ }

+ 

+ PROG="${PROG:-${0##*/}}"

+ DEBUG="${DEBUG:-}"

+ OPT_LANG="${OPT_LANG:-en}"

+ OPT_FAMILY="${OPT_FAMILY:-}"

+ OPT_ARTIFACTS_DIR="${OPT_ARTIFACTS_DIR:-}"

+ OPT_ALIAS="${OPT_ALIAS:-sans-serif}"

+ OPT_PACKAGE="${OPT_PACKAGE:-}"

+ 

+ opt=$(getopt -n "$0" --options "hvl:f:t:a:g:k:" --longoptions "help,verbose,lang:,family:,test:,artifactsdir:,alias:,package:" -- "$@")

+ eval set -- "$opt"

+ while [[ $# -gt 0 ]]; do

+ 	case "$1" in

+ 	-k|--package)

+ 		OPT_PACKAGE="$2"

+ 		shift 2

+ 		;;

+ 	-g|--alias)

+ 		OPT_ALIAS="$2"

+ 		shift 2

+ 		;;

+ 	-a|--artifactsdir)

+ 		OPT_ARTIFACTS_DIR="$2"

+ 		shift 2

+ 		;;

+ 	-f|--family)

+ 		OPT_FAMILY="$2"

+ 		shift 2

+ 		;;

+ 	-l|--lang)

+ 		OPT_LANG="$2"

+ 		shift 2

+ 		;;

+ 	-v|--verbose)

+ 		DEBUG="-v"

+ 		shift

+ 		;;

+ 	-h|--help)

+ 		msg_usage

+ 		exit 0

+ 		;;

+ 	--)

+ 		shift

+ 		;;

+ 	*)

+ 		msg_usage

+ 		exit 1

+ 	esac

+ done

+ 

+ if [ -z "$OPT_ARTIFACTS_DIR" ] || [ -z "$OPT_LANG" ] || [ -z "$OPT_FAMILY" ]; then

+ 	echo "Use: $PROG -h for help."

+ 	exit 0

+ fi

+ 

+ debug "Alias: $OPT_ALIAS"

+ debug "Family: $OPT_FAMILY"

+ debug "Lang: $OPT_LANG"

+ debug "Artifacts dir: $OPT_ARTIFACTS_DIR"

+ debug "Package name: $OPT_PACKAGE"

+ STR_TEST_DASHED=$(echo "${OPT_PACKAGE}_${OPT_ALIAS}_${OPT_LANG}" | sed -e 's/\//-/g' -e 's/ /-/g')

+ debug "Log file: $STR_TEST_DASHED.log"

+ 

+ clean_exit() {

+ 	rc=$?;

+ 	trap - SIGINT SIGTERM SIGABRT EXIT

+ 	echo "Run test $OPT_ALIAS: done. Test's exit code: $rc"

+ 	for pid in $(ps -o pid --no-headers --ppid $$); do

+ 		if [ -n "$(ps -p $pid -o pid=)" ]; then

+ 			kill -s HUP $pid

+ 		fi

+ 	done

+ 	local log_file_name="$STR_TEST_DASHED.log"

+ 	local log_file_path="$OPT_ARTIFACTS_DIR/$log_file_name"

+ 	local status

+ 	if [[ $rc -eq 127 ]]; then

+ 		status="ERROR"

+ 	elif grep -q "RESULT: WARN" "$log_file_path"; then

+ 		status="ERROR"

+ 	elif grep -q "RESULT: FAIL" "$log_file_path"; then

+ 		status="FAIL"

+ 	elif grep -q "RESULT: PASS" "$log_file_path"; then

+ 		status="PASS"

+ 	elif grep -q "FAIL" "$log_file_path"; then

+ 		status="FAIL"

+ 	elif grep -q "PASS" "$log_file_path"; then

+ 		status="PASS"

+ 	else

+ 		status="ERROR"

+ 	fi

+ 	echo "$status $OPT_ALIAS" >> "$OPT_ARTIFACTS_DIR/test.log"

+ 	mv "$log_file_path" "$OPT_ARTIFACTS_DIR/${status}-${log_file_name}"

+ 	local results="$OPT_ARTIFACTS_DIR/results.yml"

+ 	local result=$(echo $status | tr '[:upper:]' '[:lower:]')

+ 	test -f "$results" || echo 'results:' > "$results"

+ 	printf '%s\n' '' \

+ 	       "- test: $OPT_ALIAS" \

+ 	       "  result: $result" \

+ 	       "  logs:" \

+ 	       "  - ${status}_${log_file_name}" \

+ 	       >> "$results"

+ 	exit 0

+ }

+ trap clean_exit SIGINT SIGTERM SIGABRT EXIT

+ 

+ cachedir=`pkg-config --variable cachedir fontconfig`

+ tmpconfd=`mktemp --tmpdir -d fontsci.XXXXXXXX`

+ conf=$(for i in `rpm -ql $OPT_PACKAGE | grep conf.d`; do

+ 	echo "<include>$i</include>"

+ done)

+ cat <<_EOF_> $tmpconfd/fonts.conf

+ <fontconfig>

+ 	<dir>/usr/share/fonts</dir>

+ 	$conf

+ 	<cachedir>$cachedir</cachedir>

+ </fontconfig>

+ _EOF_

+ debug "Config: `cat $tmpconfd/fonts.conf`"

+ 

+ mkdir -p "$OPT_ARTIFACTS_DIR"

+ export OUTPUTFILE="$(realpath "$OPT_ARTIFACTS_DIR")/$STR_TEST_DASHED-out.log"

+ logfile="$OPT_ARTIFACTS_DIR/$STR_TEST_DASHED.log"

+ logfile="$(realpath "$logfile")"

+ exec > >(tee -a "$logfile") 2>&1

+ 

+ debug "Check family assignment"

+ res=`FONTCONFIG_FILE=$tmpconfd/fonts.conf fc-match -f "%{family[0]}" :family=$OPT_ALIAS:lang=$OPT_LANG`

+ ret=0

+ if [ "x$res" = "x$OPT_FAMILY" ]; then

+ 	echo "RESULT: PASS: $OPT_FAMILY was assigned to $OPT_ALIAS as expected"

+ else

+     echo "RESULT: FAIL: $OPT_FAMILY wasn't assigned to $OPT_ALIAS (actual result: $res)"

+     ret=1

+ fi

+ rm -rf $tmpconfd

+ 

+ exit $ret

@@ -0,0 +1,252 @@ 

+ #! /bin/bash -efu

+ 

+ debug() {

+     if [ -n "$DEBUG" ]; then

+ 	echo "$*" >&2

+     fi

+ }

+ 

+ msg_usage() {

+     cat <<_EOF_

+ Run language coverage test.

+ 

+ Usage:

+ $PROG <options>

+ 

+ Options:

+ -h, --help		Display this help and exit

+ -v, --verbose		Turn on debug

+ -l, --lang=LANG		Test LANG language coverage (default: en)

+ -p, --path=PATH		Test fonts on PATH

+ -k, --package=PACKAGE	Specify PACKAGE to obtain some information

+ -n, --name=NAME		Set NAME to store a log file.

+ -a, --artifactsdir=DIR	test environment dir to store artifacts

+ -e, --exclude=FILE	Exclude FILE to check.

+ -i, --include=FILE	Include File to check.

+ _EOF_

+ }

+ 

+ PROG="${PROG:-${0##*/}}"

+ DEBUG="${DEBUG:-}"

+ OPT_LANG="${OPT_LANG:-en}"

+ OPT_PATH=()

+ OPT_PACKAGE=()

+ OPT_ARTIFACTS_DIR="${OPT_ARTIFACTS_DIR:-}"

+ OPT_EXCLUDE=()

+ OPT_INCLUDE=()

+ OPT_NAME="${OPT_NAME:-}"

+ 

+ opt=$(getopt -n "$0" --options "hvl:p:k:n:a:e:i:" --longoptions "help,verbose,lang:,path:,package:,name:,artifactsdir:,exclude:,include:" -- "$@")

+ eval set -- "$opt"

+ while [[ $# -gt 0 ]]; do

+     case "$1" in

+ 	-n|--name)

+ 	    OPT_NAME="$2"

+ 	    shift 2

+ 	    ;;

+ 	-i|--include)

+ 	    OPT_INCLUDE+=("$2")

+ 	    shift 2

+ 	    ;;

+ 	-e|--exclude)

+ 	    OPT_EXCLUDE+=("$2")

+ 	    shift 2

+ 	    ;;

+ 	-a|--artifactsdir)

+ 	    OPT_ARTIFACTS_DIR="$2"

+ 	    shift 2

+ 	    ;;

+ 	-p|--path)

+ 	    OPT_PATH+=("$2")

+ 	    shift 2

+ 	    ;;

+ 	-k|--package)

+ 	    OPT_PACKAGE+=("$2")

+ 	    shift 2

+ 	    ;;

+ 	-l|--lang)

+ 	    OPT_LANG="$2"

+ 	    shift 2

+ 	    ;;

+ 	-v|--verbose)

+ 	    DEBUG="-v"

+ 	    shift

+ 	    ;;

+ 	-h|--help)

+ 	    msg_usage

+ 	    exit 0

+ 	    ;;

+ 	--)

+ 	    shift

+ 	    ;;

+ 	*)

+ 	    msg_usage

+ 	    exit 1

+     esac

+ done

+ 

+ if [ -z "$OPT_ARTIFACTS_DIR" ] || [ -z "$OPT_LANG" ] || [ ! -v OPT_PATH ] && [ ! -v OPT_PACKAGE ]; then

+     echo "Use: $PROG -h for help."

+     exit 0

+ fi

+ 

+ STR_TEST_DASHED=$(echo "${OPT_NAME:-$OPT_LANG}" | sed -e 's/\//-/g')

+ 

+ clean_exit() {

+     rc=$?;

+     trap - SIGINT SIGTERM SIGABRT EXIT

+     echo "Run test $OPT_LANG: done. Test's exit code: $rc"

+     for pid in $(ps -o pid --no-headers --ppid $$); do

+ 	if [ -n "$(ps -p $pid -o pid=)" ]; then

+ 	    kill -s HUP $pid

+ 	fi

+     done

+     local log_file_name="$STR_TEST_DASHED.log"

+     local log_file_path="$OPT_ARTIFACTS_DIR/$log_file_name"

+     local status

+     if [[ $rc -eq 127 ]]; then

+ 	status="ERROR"

+     elif grep -q "RESULT: WARN" "$log_file_path"; then

+ 	status="ERROR"

+     elif grep -q "RESULT: FAIL" "$log_file_path"; then

+ 	status="FAIL"

+     elif grep -q "RESULT: PASS" "$log_file_path"; then

+ 	status="PASS"

+     elif grep -q "WARN" "$log_file_path"; then

+ 	status="ERROR"

+     elif grep -q "FAIL" "$log_file_path"; then

+ 	status="FAIL"

+     elif grep -q "PASS" "$log_file_path"; then

+ 	status="PASS"

+     else

+ 	status="ERROR"

+     fi

+     echo "$status $OPT_LANG" >> "$OPT_ARTIFACTS_DIR/test.log"

+     mv "$log_file_path" "$OPT_ARTIFACTS_DIR/${status}-${log_file_name}"

+     local results="$OPT_ARTIFACTS_DIR/results.yml"

+     local result=$(echo $status | tr '[:upper:]' '[:lower:]')

+     test -f "$results" || echo 'results:' > "$results"

+     printf '%s\n' '' \

+ 	   "- test: $OPT_LANG" \

+ 	   "  result: $result" \

+ 	   "  logs:" \

+ 	   "  - ${status}_${log_file_name}" \

+ 	   >> "$results"

+     exit 0

+ }

+ trap clean_exit SIGINT SIGTERM SIGABRT EXIT

+ 

+ mkdir -p "$OPT_ARTIFACTS_DIR"

+ export OUTPUTFILE="$(realpath "$OPT_ARTIFACTS_DIR")/$STR_TEST_DASHED-out.log"

+ logfile="$OPT_ARTIFACTS_DIR/$STR_TEST_DASHED.log"

+ logfile="$(realpath "$logfile")"

+ exec > >(tee -a "$logfile") 2>&1

+ 

+ for pkg in ${OPT_PACKAGE[@]}; do

+     if ! rpm -q ${pkg} > /dev/null 2>&1; then

+ 	echo "Package isn't installed or maybe a typo: ${pkg}"

+ 	exit 127

+     fi

+     d=$(for d in $(rpm -ql ${pkg}|grep /usr/share/fonts); do

+ 	    dirname $d

+ 	done | sort | uniq | grep /usr/share/fonts/)

+     if [[ ! " ${OPT_PATH[@]} " =~ " ${d} " ]]; then

+ 	OPT_PATH+=("$d")

+     fi

+ done

+ 

+ expand_regex() {

+     local p ret=()

+     local regex="$1"

+     shift

+     debug "Expanding $regex"

+     for p; do

+ 	set +f

+ 	debug "$p: $regex"

+ 	 (cd $p;

+ 	  local x=$(find -regextype posix-egrep -regex "./$regex" -print|sed -e 's,^\./,,g')

+ 	  debug "$x"

+ 	  ret+=($x)

+ 	  set -f

+ 	  echo -n ${ret[@]}

+ 	 )

+     done

+     echo -n ${ret[@]}

+ }

+ 

+ iv=()

+ ev=()

+ x=()

+ for p in ${OPT_INCLUDE[@]}; do

+     x=$(expand_regex $p ${OPT_PATH[@]})

+     if [ "x$x" == "x" ]; then

+ 	echo "RESULT: WARN: No matches on \"$p\". maybe typo or something changed?"

+ 	continue

+     fi

+     iv=("${iv[@]}" "${x[@]}")

+ done

+ for p in ${OPT_EXCLUDE[@]}; do

+     x=$(expand_regex $p ${OPT_PATH[@]})

+     if [ "x$x" == "x" ]; then

+ 	echo "RESULT: WARN: No matches on \"$p\". maybe typo or something changed?"

+ 	continue

+     fi

+     ev=("${ev[@]}" "${x[@]}")

+ done

+ OPT_EXCLUDE=(${ev[@]})

+ OPT_INCLUDE=(${iv[@]})

+ 

+ debug "Path: ${OPT_PATH[@]}"

+ debug "Lang: $OPT_LANG"

+ debug "Artifacts dir: $OPT_ARTIFACTS_DIR"

+ debug "Exclude: ${#OPT_EXCLUDE[@]}: ${OPT_EXCLUDE[@]}"

+ debug "Include: ${#OPT_INCLUDE[@]}: ${OPT_INCLUDE[@]}"

+ 

+ contains() {

+     local e match="$1"

+     shift

+     for e; do [[ "$e" == "$match" ]] && return 1; done

+     return 0

+ }

+ 

+ debug "Check language coverage"

+ ret=0

+ set +f

+ for p in ${OPT_PATH[@]}; do

+     for i in `find $p -regex '.*/*\.\(t1\)?\(ttf\)?\(otf\)?\(ttc\)?\(pcf.*\)?\(pfa\)?'`; do

+ 	set -f

+ 	debug "$i"

+ 	if test -f $i; then

+ 	    n=`basename $i`

+ 	    set +e

+ 	    contains "$n" "${OPT_EXCLUDE[@]}"

+ 	    r=$?

+ 	    set -e

+ 	    if [ $r -eq 1 ]; then

+ 		debug "ignoring $i"

+ 		continue

+ 	    fi

+ 	    if [ ${#OPT_INCLUDE[@]} -ne 0 ]; then

+ 		set +e

+ 		contains "$n" "${OPT_INCLUDE[@]}"

+ 		r=$?

+ 		set -e

+ 		if [ $r -eq 0 ]; then

+ 		    debug "$i isn't targeted file"

+ 		    continue

+ 		fi

+ 		NOT_MATCHED=("${NOT_MATCHED[@]/$n}")

+ 	    fi

+ 	    debug "  $i"

+ 	    res=`fc-validate -l $OPT_LANG $i || :`

+ 	    if echo $res | grep -q Missing; then

+ 		echo "RESULT: FAIL: $i doesn't satisfy $OPT_LANG language coverage."

+ 		ret=1

+ 	    else

+ 		echo "RESULT: PASS: $i satisfy $OPT_LANG language coverage."

+ 	    fi

+ 	fi

+     done

+ done

+ 

+ exit $ret

@@ -0,0 +1,4 @@ 

+ ---

+ 

+ dependencies:

+   - role: str-common-init

@@ -0,0 +1,48 @@ 

+ ---

+ 

+ - block:

+   - name: language coverage

+     environment:

+       LANG: "en_US.UTF-8"

+     script: run-lang-coverage-test --lang "{{ item }}" {% if path_prefix is defined %} --path {{ path_prefix }} {% elif coverage.values is not defined or coverage[item].path_prefix is not defined %} {% else %} {{ '--path "' + (coverage[item].path_prefix | join('" --path "')) + '"' }} {% endif %} --artifactsdir "{{ remote_artifacts }}" {% if coverage.values is not defined or coverage[item].name is not defined %} {% else %} {{ "--name " + coverage[item].name }} {% endif %} {% if coverage.values is not defined or coverage[item].exclude is not defined %} {% else %} {{ '--exclude "' + (coverage[item].exclude | join('" --exclude "')) + '"' }} {% endif %} {% if coverage.values is not defined or coverage[item].include is not defined %} {% else %} {{ '--include "' + (coverage[item].include | join('" --include "')) + '"' }} {% endif %} {% if path_prefix is defined or coverage.values is defined and coverage[item].path_prefix is defined %} {% else %} {{ '--package "' + (required_packages|join('" --package "')) + '"' }} {% endif %}

+     with_items:

+     - "{{ coverage if coverage.keys is not defined else coverage.keys()|list }}"

+   - name: generic family assignment

+     environment:

+       LANG: "en_US.UTF-8"

+     when: families is defined

+     script: run-family-test --lang {{ item.lang }} --family '{{ item.family }}' --alias {{ item.alias }} --artifactsdir {{ remote_artifacts }} --package {{ package if item.package is not defined else item.package }}

+     with_items:

+     - "{{ families }}"

+ 

+   - name: Check the results

+     shell: |

+       log="{{ remote_artifacts }}/test.log"

+       if [ ! -f "$log" ]; then

+           echo ERROR

+           echo "Test results not found." 1>&2

+       elif grep ^ERROR "$log" 1>&2; then

+           echo ERROR

+       elif grep ^FAIL "$log" 1>&2; then

+           echo FAIL

+       elif grep -q ^PASS "$log"; then

+           echo PASS

+       else

+           echo ERROR

+           echo "No test results found." 1>&2

+       fi

+     register: test_results

+ 

+   - name: Set role result

+     set_fact:

+       role_result: "{{ test_results.stdout }}"

+       role_message: "{{ test_results.stderr|d('test execution error.') }}"

+       role_result_failed: "{{ test_results.stdout != 'PASS' }}"

+       role_result_msg: "{{ test_results.stderr|d('test execution error.') }}"

+ 

+   - include_role:

+       name: str-common-final

+ 

+   - name: Validate the result

+     shell: echo "test_results.stdout"

+     failed_when: test_results.stdout != 'PASS'

@@ -0,0 +1,7 @@ 

+ ---

+ 

+ role_pkgs_req:

+   - fontconfig

+   - fontconfig-devel

+   - pkg-config

+   - rsync

file added
+3567
The added file is too large to be shown here, see it at: tests/tests.yml

This enables CI tests for dejavu-fonts package.

This is just demonstrating some issues caught up by this testing. we may need to adjust test cases for a workaround until upstream is fixing. so please not merge it yet at the moment.

rebased onto ab8b3f3

4 years ago

rebased onto 0d19ae4

4 years ago

rebased onto 3c46791

4 years ago

rebased onto c9ab8f0

4 years ago

rebased onto e54c0c2

4 years ago

This is not a thorough review, just a quick and dirty first pass, because my short term priority are

  1. first, getting the new fonts packaging guidelines approved (principles and rpm automation)
  2. second, using those guidelines to update all the font packages I maintain in Fedora
  3. third, specifying all the things that need fixing fontconfig side (the parts that could not be fixed in the first point, because they require fontconfig changes)

So, that does not leave a lot of time to look at tests.

That being said:

I could not find the documentation, listing, all the tests, that will be run if this is merged

I could not find the documentation, describing, for each individual test:

  • what is the functional objective of the test
  • what is the relationship, between this objective and our packaging guidelines (the thing packagers commit to)
  • how this objective, is tested

The configuration seems to be a long list of file paths

  • this is completely wrong, because packages are not delivering file paths, or font files, but font families inside fontconfig
  • so the tests must be expressed as expected font families, available within fontconfig, when the tested package is installed regardless of the font file and fontconfig file layout, a package uses to obtain this result
  • fontconfig file and font file layout are not stable over time both upstream and Fedora side
  • I can not commit, as a packager, to keep those path stable, because it does not depend just on me (it depends on upstream and rpm macros that will change behind my back, and file changes usually result from fix requests, so asking upstreams not to change path is aking to asking upstreams not to fix problems)
  • I will not commit, as a packager, to updating long lists of file paths every time my package changes, because I have better things to do, and none of it is relevant to the functional result I give to the users of my packages

Please document what each test wants to achieve. It's a lot easier to get packages and upstream files fixed, when there is some reference text somewhere, that lists your objectives (preferably in Fedora packaging guidelines if you want packagers to do something).

I would oppose, for example, a test that tried to force font projects to use identical coverage for all the faces of their projects, because that's definitely not something they produce today, and they will definitely not produce it in the future for Fedora. So, it's an unrealistic target to reach for.

Please fix the tests so they:

  • query the rpm layer to learn what packages are generated from a rpm,
  • what font and fontconfig files are in each package.
  • and make your tests exercise the result against a target font family inside fontconfig,

That will probably require fixing fontconfig to accept direct file paths of conf files to evaluate, in addition of its default config, which is something which has been needed to fix rpm font autodeps for more than a decade.

For example:
1. the the paths, that your test config uses, will be invalidated by the guidelines update, send to FPC (this was necessary, to break completely the link between installation packages provided in the distribution, and the source packages that are used to generate those installation packagers)
2. yesterday, Plex upstream fixed a problem I reported. As a result, Plex file layout changed.

The proposed tests, as they are designed today, would punish packagers that apply packaging guidelines changes, and that get font files fix upstream. With zero user benefit, since fonts are not delivered as font files, but as font files processed by fontconfig.

This is not a thorough review, just a quick and dirty first pass, because my short term priority are

first, getting the new fonts packaging guidelines approved (principles and rpm automation)
second, using those guidelines to update all the font packages I maintain in Fedora
third, specifying all the things that need fixing fontconfig side (the parts that could not be fixed in the first point, because they require fontconfig changes)

So, that does not leave a lot of time to look at tests.

Sure. that's fine. thank you for taking a time on it.

That being said:
I could not find the documentation, listing, all the tests, that will be run if this is merged

You could find something from https://docs.fedoraproject.org/en-US/ci/

I could not find the documentation, describing, for each individual test:

what is the functional objective of the test

That is to find the potential functional issue out on upgrading etc. speaking of the dejavu specific, this test cases found out missing coverage in some variants. even if it may be a known for upstream and package maintainer as it is marked as experimental, the end users may not knows that, because they just use it as it is the part of package.
For another example, upstream may change the family name in some release for some fonts and packagers may not be aware of. this could catch it and be able to notice that they need to update fontconfig config file as well.

what is the relationship, between this objective and our packaging guidelines (the thing packagers commit to)
how this objective, is tested

Generally speaking, there is no relationship between them so far. but it may be more relevant to the common packaging guidelines or so in the future. dunno. this is optional so far anyway. but you can see if any potential issues is going to make with changes through PR say. there seems some issue on pagure side at the moment though, CI is being triggered by PR and the result can usually be available on PR UI. so this would prevent any breakage by unexpected changes if it is well written.

The configuration seems to be a long list of file paths

this is completely wrong, because packages are not delivering file paths, or font files, but font families inside fontconfig
so the tests must be expressed as expected font families, available within fontconfig, when the tested package is installed regardless of the font file and fontconfig file layout, a package uses to obtain this result

Well, that is the sort of limitation in this implementation. because fc-validate requires a file path to validate.

fontconfig file and font file layout are not stable over time both upstream and Fedora side
I can not commit, as a packager, to keep those path stable, because it does not depend just on me (it depends on upstream and rpm macros that will change behind my back, and file changes usually result from fix requests, so asking upstreams not to change path is aking to asking upstreams not to fix problems)

I'm sorry but what does "path" mean here? is it a file path? assuming that it is, how does upstream commit on it? should be more on our hands right? well, it could be probed from package perhaps. let me see.

I will not commit, as a packager, to updating long lists of file paths every time my package changes, because I have better things to do, and none of it is relevant to the functional result I give to the users of my packages

Since we aren't an upstream for a project, all of things we have in the kind of the unit test for packages could need to be rewritten for some cases. what exactly "long lists" is here isn't the file paths but test scenario. I apologize if it is just my misinterpretation, but that sounds to me like you don't want to maintain those long lists of test scenario even.

Please document what each test wants to achieve. It's a lot easier to get packages and upstream files fixed, when there is some reference text somewhere, that lists your objectives (preferably in Fedora packaging guidelines if you want packagers to do something).

Sure. well, this is just optional so far and I'm just contirubuting some efforts we've made for RHEL. if this needs to be done as a part of packaging guidelines in the future, we will need some documentation to get more understanding for font package packagers indeed.
There are no obvious measure about functional testing. that may be something we need to discuss in FontSIG for the standard testing of font packages.

I would oppose, for example, a test that tried to force font projects to use identical coverage for all the faces of their projects, because that's definitely not something they produce today, and they will definitely not produce it in the future for Fedora. So, it's an unrealistic target to reach for.

Sure. well, it may be not for everything. but we will definitely need it for default fonts at least. otherwise those fonts which is supposed to be a default for certain language may become useless in some day. if such things happens, we need to decide which fonts will be better for next default.

Please fix the tests so they:

query the rpm layer to learn what packages are generated from a rpm,
what font and fontconfig files are in each package.
and make your tests exercise the result against a target font family inside fontconfig,

That will probably require fixing fontconfig to accept direct file paths of conf files to evaluate, in addition of its default config, which is something which has been needed to fix rpm font autodeps for more than a decade.

Thanks for the comment. as I said as above, I'll try to improve that later and get back here.

1 new commit added

  • Obtain the font paths from installed packages
4 years ago

3 new commits added

  • Obtain the font paths from installed packages
  • Exclude some fonts to pass testing
  • Add CI tests
4 years ago