break `handlers.jl` into default_render, default_equality and preprocess

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

break `handlers.jl` into default_render, default_equality and preprocess #37

Closed johnnychen94 closed 4 years ago

johnnychen94 commented 4 years ago

This PR is a part of #36 that makes the overall pipeline cleaner. It's also a preparation for the introduction of @test_reference_broken and @test_reference_skip.

To avoid code duplication, codes in handler.jl are broken down into three functions:

default_render that infers the most appropriate render mode according to types of filename and actual. By default, it's BeforeAfterFull().
default_equality that infers the suitable equality compare method. By default, it's isequal.
preprocess that makes actual have the same type of reference.

oxinabox commented 4 years ago

What is the motivation of moving all this code out of its own file and intio core.jl? Won't that make it harder to find things?

From the title I thought it was going to turn handlers.jl into 3 files: rendered.jl equality_measures.jl and preprocessors.jl.

Aside: moving code and editting code makes for hard to follow diffs.

johnnychen94 commented 4 years ago

What is the motivation of moving all this code out of its own file and intio core.jl?

I'm planing to move test_reference and _test_reference from core.jl to test_reference.jl. After that, what are left in core.jl are other things related to the core pipeline.

coveralls commented 4 years ago

Coverage decreased (-1.8%) to 82.857% when pulling ffa165a1d348b10ef15489fa7ae848c04cd1179e on johnnychen94:default into dcba05aec09643a995d308072f5100cdfbca042a on Evizero:master.

johnnychen94 commented 4 years ago

@oxinabox I've tried to minimize the differences for you to review.

Since all codes in handlers.jl are removed and rewrite using default_rendermode, default_equality and preprocess. It's unavoidable hard to compare and review the diff. I've double-checked them and I'm confident about the logic here.

It comes to an end of my vacation, I need to focus on my school stuff first and then I'll come back continue it.

Since you prefer holding things apart in separate files, I'll move things related to RenderMode to render.jl when this PR is ready to be merged.

After this PR:

Documentation will be updated
implement Test-corresponding macros like @test_reference_broken, @test_reference_skip

johnnychen94 commented 4 years ago

mind to take a look at it? 👀@oxinabox

johnnychen94 commented 4 years ago

The last three commits do a trivial organization work breaking core.jl into different files. You can review the difference commit by commit.