Clear-Bible / macula-hebrew

Syntax trees, morphology, and linguistic annotations for the Hebrew Bible
Other
36 stars 9 forks source link

Test framework #79

Closed themikejr closed 1 year ago

themikejr commented 1 year ago

This PR does the following:

themikejr commented 1 year ago

There are currently two failing tests that ironically prevent this from being merged:

  1. number of words in lowfat
  2. number of words in macula_hebrew.tsv
jacobwegner commented 1 year ago

@themikejr wrote:

There are currently two failing tests that ironically prevent this from being merged:

  1. number of words in lowfat
  2. number of words in macula_hebrew.tsv

Given that the missing words are a known issue (https://github.com/Clear-Bible/symphony-team/issues/207), I would suggest we either:

1) Update the expected word counts to match what is actually in the lowfat / TSVs (and then the "fix" for #207 would include updating the tests so they pass again with the new counts) 2) Mark them using the xfail marker; when we resolve #207, we can remove the marker

themikejr commented 1 year ago

@themikejr wrote:

There are currently two failing tests that ironically prevent this from being merged:

  1. number of words in lowfat
  2. number of words in macula_hebrew.tsv

Given that the missing words are a known issue (Clear-Bible/symphony-team#207), I would suggest we either:

  1. Update the expected word counts to match what is actually in the lowfat / TSVs (and then the "fix" for #207 would include updating the tests so they pass again with the new counts)
  2. Mark them using the xfail marker; when we resolve #207, we can remove the marker

Thanks for the suggestions @jacobwegner. I went with the latter so that we don't lose the encoded expectation of the correct number of words.

themikejr commented 1 year ago

I believe this is ready for review now.

jacobwegner commented 1 year ago

(I'm going to go ahead and merge this in because it will be useful to have in place for https://github.com/Clear-Bible/symphony-team/issues/193)