acl-anthology icon indicating copy to clipboard operation
acl-anthology copied to clipboard

Test Incorporation

Open xinru1414 opened this issue 3 years ago • 1 comments

Inspired by the recent incidents of change-in-some-scripts resulting in the breaking of other scripts and back-and-forth reversing the change (see https://github.com/acl-org/acl-anthology/commit/8500a12307de69be9b799308dbdf7e57841d9537#r67683715, https://github.com/acl-org/acl-anthology/pull/1836/commits/50531e243ea5b9842cb359c75a2f8851a1b42002 and discussion in https://github.com/acl-org/acl-anthology/pull/1833#issuecomment-1055813438), it seems like we could really use some tests.

While trying to use pytest package for the tests, I encountered a problem. The version oftexsoup package we currently use is ancient and isn't compatible with pytest. https://github.com/acl-org/acl-anthology/blob/56145deed72d4b3a43788e5ea7bfe200b12cd2d8/bin/requirements.txt#L18

Upgradinig the texsoup version will solve the problem and I think is the best solution (unless for some reason we have to work with the ancient version). Before I dive into changing the code (https://github.com/acl-org/acl-anthology/blob/master/bin/anthology/texmath.py):

  • I'm not terribly familiar with texsoup so upgrading the codebase isn't trivia. Any insights/tips that could speed up the process are greatly appreciated!
  • Any other suggestions/solutions?

@mjpost @mbollmann

xinru1414 avatar Mar 02 '22 22:03 xinru1414

Thanks for the initiative!

In theory, having the automated build checks should catch most serious problems already, but it obviously doesn't catch all of them as we could see here. Maybe we could prioritize writing tests for things like ingestion-related functionality, which won't necessarily be covered by the build check.

Re texsoup, the version is fixed to <=0.1.4 because the 0.2 release was not backwards-compatible and broke our build, see #262. I have not looked into what would be required to upgrade texsoup to a newer version.

mbollmann avatar Mar 02 '22 22:03 mbollmann