CI fails for order of rendered HTML directives

[miketheman]

After #231 was merged, the main branch tests fail due to the order of the HTML directives used.

See https://github.com/pypa/readme_renderer/runs/6064288793?check_suite_focus=true#step:5:27

Here's the list of installed deps:

attrs==21.4.0,bleach==5.0.0,cffi==1.15.0,cmarkgfm==0.8.0,docutils==0.18.1,iniconfig==1.1.1,packaging==21.3,pluggy==1.0.0,py==1.11.0,pycparser==2.21,Pygments==2.11.2,pyparsing==3.0.8,pytest==7.1.1,readme-renderer @ file:///home/runner/work/readme_renderer/readme_renderer/.tox/.tmp/package/1/readme_renderer-34.0.tar.gz,six==1.16.0,tomli==2.0.1,webencodings==0.5.1

And the failure seen (abbreviated):

>       assert render(md_markup, variant=variant) == expected
E       assert '<ul>\n<li><i.../li>\n</ul>\n' == '<ul>\n<li><i.../li>\n</ul>\n'
E         Skipping 110 identical trailing characters in diff, use -v to show
E           <ul>
E         - <li><input disabled type="checkbox"> Valid unchecked checkbox</li>
E         ?           ---------
E         + <li><input type="checkbox" disabled> Valid unchecked checkbox</li>
E         ?                           +++++++++
E         - <li><input checked disabled type="checkbox"> Valid c...
E         
E         ...Full output truncated (2 lines hidden), use '-vv' to show

and

>       assert render(md_markup, variant=variant) == expected
E       assert '<p><img src=...cat">\n</p>\n' == '<p><img alt=...20%">\n</p>\n'
E         - <p><img alt="Image of Yaktocat" src="https://octodex.github.com/images/yaktocat.png"></p>
E         + <p><img src="https://octodex.github.com/images/yaktocat.png" alt="Image of Yaktocat"></p>
E           <p align="center">
E         -     <img alt="Image of Yaktocat" height="100px" src="https://octodex.github.com/images/yaktocat.png" width="20%">
E         +     <img src="https://octodex.github.com/images/yaktocat.png" width="20%" height="100px" alt="Image of Yaktocat">
E           </p>

As we can see in both examples, the attribute of either the input or img tag has been changed from the expected test order, thus failing.

The "easy fix" is to update the test case's expectations, but I'd like to understand why it failed in the first place, when no other (apparent) changes were made.

[miketheman]

Digging deeper...

Inspecting the test run that the branch ran on the PR, we can see this set of deps:

attrs==21.4.0,bleach==4.1.0,cffi==1.15.0,cmarkgfm==0.8.0,docutils==0.18.1,iniconfig==1.1.1,packaging==21.3,pluggy==1.0.0,py==1.11.0,pycparser==2.21,Pygments==2.11.2,pyparsing==3.0.7,pytest==7.1.1,readme-renderer @ file:///home/runner/work/readme_renderer/readme_renderer/.tox/.tmp/package/1/readme_renderer-34.0.tar.gz,six==1.16.0,tomli==2.0.1,webencodings==0.5.1

The only one that stands out is bleach==4.1.0 in the PR vs bleach==5.0.0, which was released on April 7th, 2022.

The first entry on that page is:

• clean and linkify now preserve the order of HTML attributes. Thank you, @askoretskly! (#566)

Leading to mozilla/bleach#566 - which also explains why it was picked for a major release, since it technically breaks backwards compat, which it has here.

Question: This project expresses at least bleach 2.1.0 - if we were to update the tests to match the current 5.x behavior, would we need to bump that as well?

[di]

Thanks for catching this. I think this is a small enough "breaking change" that we don't need to restrict ourselves to bleach>=5.0, and we should just update our tests to match the new behavior.

Perhaps we could also introduce a constraints file when installing test dependencies to add constraints like this, which won't have significant user-facing changes if different versions are installed, but would make our tests fail?

[miketheman]

... we should just update our tests to match the new behavior.

Incoming PR. 🎉

Perhaps we could also introduce a constraints file when installing test dependencies to add constraints like this, which won't have significant user-facing changes if different versions are installed, but would make our tests fail?

Before fixing the tests, I took a look at this - it wasn't something I'd done before, so thanks for the link!

Looks like the tox support is still marked as experimental - https://tox.wiki/en/latest/example/basic.html#depending-on-requirements-txt-or-defining-constraints
Now, that was back in 1.6.1 and we're at 3.25, so it's entirely possible that it's no longer experimental and the docs lag, but I was unable to finagle tox configuration to install bleach==4.1.0 from a constraints.txt file, but pip did the Right Thing with a pip install . -cconstraints.txt.

It's possible that tox has a bug here, or that our installation order expectations is different, since we express install deps in setup.py and tox doesn't take constraints into account at the same time? 🤔