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

better structure test suite to make clear what's required of CSL implementers #17

Closed
bdarcus opened this issue May 27, 2020 · 16 comments
Closed
Assignees

Comments

@bdarcus
Copy link
Member

bdarcus commented May 27, 2020

This arises from #13 and #16. What I concluded on the former issue is:

... the best immediate path on the test-suite is to create a separate subdirectory and start to move tests like this to that, and while doing it, add some metadata to these that tag them as such, maybe with some description. We could start with this one.

What I'm contemplating is we need to add three additional parameters or sections to the tests; perhaps:

  1. version (with values like "1.0", "1.1", "1.0-M"; that sort of thing)
  2. description (ideally we write this in spec language)
  3. section (something to group tests together)

My thought is that we do this in such a way that a processing spec can (at least mostly) be auto-assembled without additional work.

If we do this right, the tests themselves become definitive, very clear, and the documentation can be directly tied to them.

While it would take time, this seems the quickest path to addressing the concerns in the linked issue, in iterative ways that fit existing resource constraints.

I realize it's more complicated than this (for example, multiple tests will often describe one larger processing logic), but hopefully those are solvable problems.

Not sure about whether subdirs would be necessary with this approach, but it might wise to at least separate CSL-M?

@citation-style-language/schema-pr-reviewers

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

For illustration, this:

>>==== MODE ====>>
citation
<<==== MODE ====<<

... could become:

>>==== VERSION ====>>
1.0
<<==== VERSION ====<<
>>==== DESCRIPTION ====>>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. In cursus ante erat, 
tempus ornare dolor gravida et. Morbi efficitur quam ac varius laoreet. 
Nullam rhoncus est in diam rhoncus ultricies. Nam ligula turpis, consequat 
non dapibus in, vulputate in est. Praesent porttitor ac quam vel convallis. 
<<==== DESCRIPTION ====<<
>>==== SECTION ====>>
locators
<<==== SECTION ====<<
>>==== MODE ====>>
citation
<<==== MODE ====<<

... or maybe we allow a YAML header of something like this (standard, and much more compact):

---
name: some test
mode: citation
versions:
   - 1.0
   - 1.1
section: locators
description: >
            Lorem ipsum dolor sit amet, consectetur adipiscing elit. In cursus ante 
            erat, tempus ornare dolor gravida et. Morbi efficitur quam ac varius laoreet. 
            Nullam rhoncus est in diam rhoncus ultricies. Nam ligula turpis, consequat 
            non dapibus in, vulputate in est. Praesent porttitor ac quam vel convallis.
---

The above would say the same behavior applies across both 1.0 and 1.1.

Perhaps we have a directory structure that would correspond to generated documents (though current file names already have this sort of structure); like:

/tests
     /introduction
     /citations
     /bibliographies
     /names

It does seems like the existing tests have enough info to largely automate this conversion as a first pass (version can be derived from embedded CSL, for example), and then human could add anything else as time permits, or they need to deal with a particular test or test section.

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

Hmm .... here's a problem with automating a key part of this; was hoping we could somehow test on the embedded CSL, but it turns out ...

> rg -c 'version="1.0"' processor-tests/humans | wc -l 
869

E.g. it seems that there's no version distinction based on csl vs csl-m?

@denismaier
Copy link
Member

Maybe there are no csl-m tests in this repo?
At the citeproc-js repo you'll find test fixtures for csl-m, e.g. this one.

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

That would be great; if effectively there were only a handful that slipped through?

@denismaier
Copy link
Member

... or maybe we allow a YAML header of something like this (standard, and much more compact):

Have you had a look a jest-csl?
That is used for testing styles, but the syntax is instructive.

Concering

versions:
   - 1.0
   - 1.1

Maybe something like this:

versions:
   min: 1.0
   max: 1.1

???

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

No, but that looks really promising! It uses YAML.

Where the hell is @cormacrelf anyway; seems we could use his help ;-)

Not just because of jest (and because I look forward to a complete citeproc-rs!), but because he's the most recent developer to work his way through a full CSL implementation.

Oh, and sure on your suggestion; I just floated a concrete strawman to get the ball rolling. Might be jest has some better ideas.

@denismaier
Copy link
Member

That would be great; if effectively there were only a handful that slipped through?

I have to admit that I don't always understand which features require csl-m version attribute. Some csl-m features seem to work also with version="1.0.".

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

@bdarcus
Copy link
Member Author

bdarcus commented May 27, 2020

His "ignore" list, with explanation:

https://github.com/cormacrelf/citeproc-rs/blob/cd73f28945a980e984e73163f6f59e513336c570/crates/citeproc/tests/data/ignore.txt

One possibility immediately, then, is to add his list to our test directory, with our preface?

bdarcus added a commit that referenced this issue May 27, 2020
Until #17 is addressed, this borrows the ignore.txt list from the
citeproc-rs project, with explanation, to provide guidance to
implementers on which tests they should ignore.
bdarcus added a commit that referenced this issue May 27, 2020
Until #17 is addressed, this borrows the ignore.txt list from the
citeproc-rs project, with explanation, to provide guidance to
implementers on which tests they should ignore.
bdarcus added a commit that referenced this issue May 28, 2020
Until #17 is addressed, this borrows the ignore.txt list from the
citeproc-rs project, and adds two additional tests identified in #16.

This list identifies those tests that rely on undocumented modes in 
citeproc-js. 

The contents are simply one filename per line, for easy processing.
@bdarcus
Copy link
Member Author

bdarcus commented May 29, 2020

I'm going to close this for now, as I think we've done what we can quickly.

The bigger and broader changes (though it should be feasible to automate a lot of what I suggest I think) will be necessary as we move towards v1.1, but those will have to wait.

@bdarcus bdarcus closed this as completed May 29, 2020
bdarcus added a commit that referenced this issue Jun 13, 2020
Partially addresses #17 and #25, this adds a "VERSION" field to the
processor.py script.

Syntax for the field value is: [version]:[tag].
@fbennett
Copy link
Member

fbennett commented Jun 13, 2020

To confirm, tests that exercise CSL-M style code have a version of 1.1mlz1, and reside in the citeproc-js repo. Some features of CSL-M may be recognized by that processor in CSL mode, where they don't conflict with CSL behavior and I was too lazy to disable them in the processor code.

@bdarcus
Copy link
Member Author

bdarcus commented Jun 13, 2020

So at this point, @fbennett, the only issues in the files in this repo may be where spec and test need to be aligned?

And are you saying in the cslm test repo, you are also using a "version" variable?

@fbennett
Copy link
Member

fbennett commented Jun 13, 2020

There may still be some things in there that need weeding out. If you come across any culprits, let me know and I'll move them out to the other repo.

On version markers and description, YAML would be good. Also, it might be helpful to both style and processor developers to assign tags to the fixtures, to indicate features that each is meant to exercise, maybe to indicate the "level" of CSL complexity reflected in the test, and to flag things that may want editorial attention (such as fixtures with unnecessarily verbose CSL). I guess something like that would start with a controlled and curated list of tag names.

@bdarcus
Copy link
Member Author

bdarcus commented Jun 14, 2020 via email

@fbennett
Copy link
Member

If a team dug into it, the choices of syntax and content constraints should be made carefully, of course.

@bdarcus
Copy link
Member Author

bdarcus commented Jun 14, 2020

So sounds like maybe I should add tags and description to the PR. I don't feel comfortable modifying beyond that, but it leaves room for others to build on it later (for example, converting to YAML, or whatever).

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

No branches or pull requests

5 participants