JuliaTesting / ReferenceTests.jl

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

By default, use UnicodePlots for plot reference test #90

Closed johnnychen94 closed 3 years ago

johnnychen94 commented 3 years ago

GUI plot backends might have issues across different platforms, UnicodePlots.jl is a lightweight and generic plot backend in pure Julia so it might be worth introducing it as a dependency to ReferenceTests, and makes it the default backend when we do plot reference test.

juliohm commented 3 years ago

It is good to keep in mind that the UnicodePlots.jl backend has limited attributes support in Plots.jl and that we rely a lot on the most complete backend in our visual tests, which is GR.jl

On Thu, Apr 22, 2021, 03:28 Johnny Chen @.***> wrote:

GUI plot backends might have issues across different platforms, UnicodePlots.jl https://github.com/Evizero/UnicodePlots.jl is a lightweight and generic plot backend in pure Julia so it might be worth introducing it as a dependency to ReferenceTests, and makes it the default backend when we do plot reference test.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JuliaTesting/ReferenceTests.jl/issues/90, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3NJ5MMUI4L2M5WYPZTTJ66XHANCNFSM43LYZCHQ .

oxinabox commented 3 years ago

Does saving to PNG work on UnicodePlots? Main point of that test is to test we can save a plot to PNG and then compare it.

johnnychen94 commented 3 years ago

Does saving to PNG work on UnicodePlots?

Not yet, but if it's agreed I can try to work on this.

It is good to keep in mind that the UnicodePlots.jl backend has limited attributes support in Plots.jl and that we rely a lot on the most complete backend in our visual tests, which is GR.jl

I'll take this into consideration; making UnicodePlots the default plot backend still allows you to switch to other backends (might need some appropriate checks and handles). The general idea here is: test cases should be as simple as they can so that it's backend/platform-agnostic and thus more reliable; yes there can be some advanced attributes not supported by UnicodePlots.jl, but for CI regression test I believe it's sufficient for many of the plot setups.

Unless we do the plot reference test only on ubuntu, which I don't think is a good permanent strategy, I can't think of another stable solution to get the green checks back.

oxinabox commented 3 years ago

Unless we do the plot reference test only on ubuntu, which I don't think is a good permanent strategy, I can't think of another stable solution to get the green checks back.

I think we could look into using GRUtils.jl rather than Plots.jl. And generally exercises similar code paths. I think it might not have this problem.

Long term this is a actual bug in Plots/GR so it getting fixed is the solution. Til then I don't mind only testing this in 1 OS. Our logic is OS independent. It is the other package doing the savings and loading that had the bug in some OSes

juliohm commented 3 years ago

I agree with @oxinabox , it is better to have ReferenceTests.jl working fine with any GR plot in a single OS than a limited experience with UnicodePlots.jl in multiple OSes. I reiterate that I rely heavily on GR to test all my packages which depend on ReferenceTests.jl now. Things break here and there because of the annoying Plots.jl situation, but it is fine.

johnnychen94 commented 3 years ago

Reasonable enough for me. I'll close this issue (and maybe open a new one) as using UnicodePlots for saving is not what we actually want.