erikedin / Behavior.jl

Tool for Behavior Driven Development in Julia
Other
25 stars 3 forks source link

Display paths of files containing duplicate steps #106

Closed m-charlton closed 2 years ago

m-charlton commented 2 years ago

Problem

File path(s) of step files, which contain duplicate steps, are not displayed in error message.

How to reproduce

In a 'steps' file(s) duplicate a step used in an existing scenario.

# features/steps/ExperimentSteps.jl

@given("a machine filled with coffee beans") do context
    context[:machine] = Machine(coffee=5.0)
end

@given("a machine filled with coffee beans") do context
end

Run the tests

(MPackage) pkg> test
     Testing MPackage
     ...
     Testing Running tests...

Feature: Making Coffee
  Scenario: Making a regular coffee
    Given a machine filled with coffee beans

        Multiple matches found:
          In <no filename>
          In <no filename>

     When a user makes a cup of coffee
     ...
                         | Success | Failure
  Feature: Making Coffee | 0       | 1

FAILURE

Expected to see the file path(s) of the steps files which contain the duplicate rules - instead: <no filename>.

Solution

The problem looks to be in the readstepdefinitions! function in src/engine.jl. The constructor of FromMacroStepDefinitionMatcher takes an optional filename argument which has a default value of <no filename>. A filename argument is not provided.

With the 'fix' we now get the full paths of the offending step files.

     Multiple matches found:
        In /home/michael/projects/scratch/MyProject/MPackage/features/steps/ExperimentSteps.jl
        In /home/michael/projects/scratch/MyProject/MPackage/features/steps/ExperimentSteps.jl

Observations

Despite the multiple definitions being in one file, in the above example, the path is printed twice. Perhaps the offending file name should be printed once? Although an argument against this is that the existing approach gives an indication of how many duplicates there are.

The absolute file paths are printed. Printing long path names can be ugly and take up screen real estate. Perhaps only the path from where the tests are run should be printed?

     Multiple matches found:
        In MPackage/features/steps/ExperimentSteps.jl
        In MPackage/features/steps/ExperimentSteps.jl

Although I can see a number of problems here:

The biggest argument against the above is printing out the full path gives an unambiguous answer as to where the problem(s) lies.

erikedin commented 2 years ago

Thanks, this is one of those smaller things I've never gotten around to doing.

Good observations regarding absolute vs. relative paths. I'm making an effort to keep dependencies to a minimum, so I'd ideally prefer not to add that dependency. There is the concept of "root path" when running the tests. All step definitions files are found relative to that, so possibly one could print them relative that path. I think that would make the most sense, but it's also not critical in my opinion. Absolute paths are always unambiguous, which is nice.

I don't think having the paths printed twice is necessarily bad. Looking at it now, I realize we should probably also print the line numbers where the definitions are found in the file. Then having two paths becomes important again, as they would have different line numbers, like

     Multiple matches found:
        In MPackage/features/steps/ExperimentSteps.jl:17
        In MPackage/features/steps/ExperimentSteps.jl:42

However, I'm not sure if we actually have that information stored somewhere in the definitions. I know I looked at it when I first made this package, but that was for Julia 0.4, years ago, so a lot has probably changed since then. I'll make an issue for that as an improvement.

m-charlton commented 2 years ago

Line information for step definitions

I think you're right. It looks as if line information is held in a StepDefinitionLocationinstance. This is set, to '0', in the step_definition_(...) function. Both are in src/stepdefinitions.jl.

As an aside, I think you can safely delete the call to makedescritptionregex(description) as the descriptionregex variable is not used in this function.


function step_definition_(definition::Expr, description::String)
    # The step definition function takes a context and executes a bit of code supplied by the
    # test writer. The bit of code is in $definition.
    definitionfunction = :((context, vars) -> $definition(context, vars...))
    descriptionregex = makedescriptionregex(description)
    quote
    ...
end

The makedescriptionregex(...) function is, however, called in the StepDefinition constructor.