search

JuliaTesting / ReferenceTests.jl

Utility package for comparing data against reference files
https://juliatesting.github.io/ReferenceTests.jl/latest/
Other
83 stars 15 forks source link

drop `ImageInTerminal` #117

Closed t-bltg closed 2 years ago

t-bltg commented 2 years ago

Fix https://github.com/JuliaTesting/ReferenceTests.jl/issues/116, all tests pass locally.

Needs https://github.com/JuliaImages/ImageInTerminal.jl/pull/71 and https://github.com/JuliaImages/XTermColors.jl/pull/8.

Ping @johnnychen94 for review, since I cannot request for a review here (not a member of JuliaTesting org).

oxinabox commented 2 years ago

This PR would be much better if it was broken into two. One which supports ImageInTerminals v0.5 and updates all reference images and one containing all other changes.

t-bltg commented 2 years ago

This PR would be much better if it was broken into two.

Agreed, will split (EDIT: see https://github.com/JuliaTesting/ReferenceTests.jl/pull/118).

johnnychen94 commented 2 years ago

Maybe we'd just switch to XTermColors.ascii_show? For test utils like ReferenceTests, it actually doesn't make much sense to override the default display or Base.show methods -- it might have unexpected side-effects. For instance, if people use Documenter.doctest in test/runtests.jl, it will be ruined.

t-bltg commented 2 years ago

Maybe we'd just switch to XTermColors.ascii_show

This is very clever and benefits from the rework / splitting of ImageInTerminal 0.5 (credits to @johnnychen94 for the big picture):

  1. it avoids ImageInTerminal side effects e.g. overriding Base.show;
  2. it reduces dependencies, thus improving load times;
  3. it solves the circular dependency at the origin of https://github.com/JuliaTesting/ReferenceTests.jl/issues/116.
johnnychen94 commented 2 years ago

@t-bltg let me know when you're ready for ReferenceTests v0.10 release.

t-bltg commented 2 years ago

When you want, so that we can go forward on https://github.com/JuliaImages/ImageInTerminal.jl/pull/71.