feat: Separation of concerns #19
Conversation
Currently there is only one way to load the tests; may well want to tweak this.
matatk
changed the title
Found, at last, a great little package to do this; thanks, https://github.com/zakhenry/embedme :-)
…ses as syntax errors
…seem to affect rendering)
* Fix the spacing (indentation) of the note on limitations of the combined tests. * Tweak the wording and non-visible spacing around the code example.
matatk
force-pushed
the
separation-of-concerns
branch
from
6b907df
to
8cad4bc
Compare
* Simplify the test suite info section generally. * Remove the first heading within "Test suite info". * Clarify the two formats of test suite provided. * Obfuscate example path to fixture (was Mac-specific)
Finding clarity is harder than was anticipated :-).
matatk
force-pushed
the
separation-of-concerns
branch
from
174290a
to
c426c1e
Compare
|
@xi I was wondering if you might have the cycles to try this out? |
|
Looks much cleaner now! I particularily like that installations does no longer pull in dependencies that I do not actually need. A minor nitpick is that I would prefer a flat list of landmarks instead of the "contains" structure in the expectations. Other than that this is great! |
|
@xi thanks for trying it and getting back to me so quickly. Glad to hear the changes are helpful :-). It was satisfying to remove the run-time dependencies :-). Regarding “contains”: I did that because I’m anticipating adding headings and articles to the test suite at some point, and I thought that this structure would be clearer for when that happens. It describes the expectations without containing any implementation details and, being a tree, it mirrors the DOM's structure. (Since you mention it, though, I am thinking about this again.) The Landmarks extension uses a different format too: everything is in a list and a “depth” field indicates nesting. I wrote and tested a converter for Landmarks and a converter for a11y-outline—might that be of use? There are tests for the converters. I'm planning to move all of that, and more, to a separate repo that's more experimental. |
|
If you test for the correct hierarchy you also have to deal with But that is really not related to this pull request. Should I open a separate issue? |
|
@xi that does sound to me like a separate issue, I'm not sure that I am understanding correctly—if we're only testing for the landmarks hierarchy at the moment, I'm not sure that My understanding is that a11y-outline/aria-api uses some sort of tree structure internally too (that’s what I used for the basis of the converter tests). I may be misunderstanding though. I agree that’s out-of-scope for now, though. I was wondering if using the word “combined” to describe the combined HTML fragment tests is sub-optimal. It may sound like they represent a complete set of tests, where in fact two are missing. I thought maybe calling them “fragment” tests (and changing the directory name too) instead may be clearer. Do you have a strong preference either way? |
|
It occurred to me that the "combined" tests could be changed to "fragment" ones later on, so it's not key to this PR. I will need to make a new repo for the code that this PR removes from this one, and aim to do that soon, so I can merge this. Thanks again @xi for your feedback so far :-). |
So that the PR can be merged.
matatk
changed the title
|
Finally merged, tagged and released :-). This is a breaking change, but as we're still at 0.x—i.e. in initial development—I just bumped to 0.4. |
Include just the test suite and minimal support code in this repository.
TODO:
[ ] Try removingt.end()calls (from the docs, they may not be necessary).Closes #18