Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow config for rerun location as line number for shared or nested examples #3085

Conversation

kykyi
Copy link
Contributor

@kykyi kykyi commented May 12, 2024

Related to #2119 (comment)

What is the change?

The out of the box behaviour has not changed, but a new configuration option has been added ✨

config.use_line_number_for_failed_spec_location_rerun

The end result is an output which is easier for humans and non-rspec computers (like vim or vscode) to jump to the location of the failure 🧑‍💻

Say you use a shared_example (or setup your specs inside a loop):

# spec/temp_spec.rb

RSpec.describe do
    shared_examples_for 'a failing spec' do
        it 'fails' do
            expect(1).to eq(2)
        end
    end

    context 'the first context' do
        it_behaves_like 'a failing spec'
    end

    context 'the second context' do
        it_behaves_like 'a failing spec'
    end
end

The default output will look something like:

rspec './spec/temp_spec.rb[1:1:1:1]' #  the first context behaves like a failing spec fails
rspec './spec/temp_spec.rb[1:2:1:1]' #  the second context behaves like a failing spec fails

This PR adds the ability to instead print the failure line number:

rspec './spec/temp_spec.rb:10' #  the first context behaves like a failing spec fails
rspec './spec/temp_spec.rb:14' #  the second context behaves like a failing spec fails

What if the shared example is defined in another file?

As you can see in the above, the failure line number is the line where it_behaves_like is called from, and this is the case if the shared_example is defined elsewhere (see new specs).

Note that there is no 'ideal' situation which will match the perfect amount of information in the CLI output to the ease of re-running and subsequent debugging of specs. The current solution allows for easy re-running of tests, but is not human readable, and the solution implemented in this PR is still easily re-runnable, and a bit more readable, but at the cost of requiring a little bit more inspection in your text editor 😄

Comment on lines 72 to 78
Failed examples:

rspec './local_shared_examples_spec.rb[1:1:1:1]' # the first context behaves like a failing spec fails
rspec './local_shared_examples_spec.rb[1:1:1:2:1]' # the first context behaves like a failing spec when you reverse it still fails
rspec './local_shared_examples_spec.rb[1:2:1:1]' # the second context behaves like a failing spec fails
rspec './local_shared_examples_spec.rb[1:2:1:2:1]' # the second context behaves like a failing spec when you reverse it still fails
MSG
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This syntax is a bit gross but allows for the whole failure segment to be asserted on which is nice

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use a pattern elswhere where we define a dedent helper and add a "pillar" to mark text content, I've add this as a suggestion but I did it in the browser so excuse any typos.

@kykyi
Copy link
Contributor Author

kykyi commented May 22, 2024

Any thoughts @JonRowe 🙏

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay reviewing this, personally I would have preferred to see a fully configurable re-run format, but this is probably a reasonable compromise.

I do want to see the specs cleaned up slightly using our dedent pattern and I feel strongly that the config name should have force in it because we do include line numbers where possible, and its not the actual spec location if we always print a line number, it'll often be the group location in the case of shared examples.

lib/rspec/core/configuration.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Outdated Show resolved Hide resolved
spec/rspec/core/configuration_spec.rb Outdated Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Show resolved Hide resolved
spec/integration/location_rerun_spec.rb Show resolved Hide resolved
Comment on lines 72 to 78
Failed examples:

rspec './local_shared_examples_spec.rb[1:1:1:1]' # the first context behaves like a failing spec fails
rspec './local_shared_examples_spec.rb[1:1:1:2:1]' # the first context behaves like a failing spec when you reverse it still fails
rspec './local_shared_examples_spec.rb[1:2:1:1]' # the second context behaves like a failing spec fails
rspec './local_shared_examples_spec.rb[1:2:1:2:1]' # the second context behaves like a failing spec when you reverse it still fails
MSG
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use a pattern elswhere where we define a dedent helper and add a "pillar" to mark text content, I've add this as a suggestion but I did it in the browser so excuse any typos.

@kykyi
Copy link
Contributor Author

kykyi commented Jun 14, 2024

Thanks for the review @JonRowe! Seems like all of the comments except the config name are lint related which is amusing for rubocop-core 😄

Should we set up cops for these leveraging rubocop-rspec? Or does that create some sort of cyclic dependency issue?

fully configurable re-run format

I think getting this PR in is an improvement and happy to add a more configurable solution in a follow up PR. I am not sure I fully understand what a fully configurable re-run format means, ie what are some of the options you would like to be able to be configured?

kykyi and others added 3 commits June 14, 2024 15:51
@kykyi kykyi requested a review from JonRowe June 14, 2024 06:04
@pirj
Copy link
Member

pirj commented Jun 14, 2024

We plan the unification of RuboCop config, but this is not trivial with many repos. We postponed it to after going monorepo and dropping some legacy code in RSpec 4. But it’s also not a priority, see rspec/rspec-metagem#61

it_behaves_like 'a failing spec'
end
end
"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you convert the files to use the same unident trick, should neatedn them up to as theres a stray miasligned "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the indents manually @JonRowe and have used write_file_formatted which calls unindent just to use the existing util 👌

https://github.com/kykyi/rspec-core/blob/39f4fdfeea8ff6c70314ba385a47f20fc370cd2c/spec/support/aruba_support.rb#L72

@kykyi kykyi requested a review from JonRowe June 14, 2024 22:20
@kykyi
Copy link
Contributor Author

kykyi commented Jul 9, 2024

Hey @JonRowe keen to get this one merged if we can 🙏

@JonRowe JonRowe merged commit bd01ac0 into rspec:main Jul 9, 2024
29 of 30 checks passed
JonRowe added a commit that referenced this pull request Jul 9, 2024
@JonRowe
Copy link
Member

JonRowe commented Jul 9, 2024

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants