Improve the RSpec Queue Mode runner log output (add seed)#178
Conversation
shadre
force-pushed
the
improve-log-output
branch
7 times, most recently
from
76ca411 to
6fde7dd
Compare
ArturT
left a comment
ArturT
left a comment
There was a problem hiding this comment.
I checked the code and left some minor comments.
I'm going to do manual testing and then I will do the final code review.
shadre
force-pushed
the
improve-log-output
branch
2 times, most recently
from
78f35ed to
b84a3db
Compare
ArturT
left a comment
ArturT
left a comment
There was a problem hiding this comment.
Looks good. I left some minor comments on what could be improved. It's up to you.
While I was manually testing these changes, I added bin scripts that I was using for testing. Here is related PR: KnapsackPro/rails-app-with-knapsack_pro#40
shadre
force-pushed
the
improve-log-output
branch
5 times, most recently
from
8795fdf to
8b4109a
Compare
| it { expect(subject).to eq 'random' } | ||
|
|
||
| context 'with the seed' do | ||
| let(:cli_args) { ['--order', 'rand:123456'] } |
There was a problem hiding this comment.
should there be random:123456 instead of 'rand:123456'?
|
|
||
| context 'when @@used_seed has been set' do | ||
| let(:used_seed) { '8333' } | ||
| let(:logged_rspec_command_matcher) { /#{args.join(' ')} --seed #{used_seed} \"#{all_test_file_paths.join(' ')}\"/ } |
There was a problem hiding this comment.
Sometimes I wonder what to do in a situation like this.
It's easier to use programming in the spec to generate the expected string. I often do it because I like the convenience. But on the other hand, we can make mistakes in the spec as well. Who is going to test the spec itself? - once, a friend asked me :)
I had to put some mental effort into figuring out what we actually expected as a string here. It wasn't clear from reading the code.
I see one potential problem. For now, we assume let(:all_test_file_paths) { ['a_spec.rb'] } has one spec only.
If there would be more specs, then the \"#{all_test_file_paths.join(' ')}\" code does not generate a string that we actually should expect. Each spec file should be inside of apostrophes. For example, for 2 specs it should be "a_spec.rb" "b_spec.rb". It's needed to ensure RSpec can run spec files from a directory with a space in the name.
Currently your coude would generated incorrect string "a_spec.rb b_spec.rb".
You may consider using the expected string explicitly here so that reading the code is easier.
But I'm leaving it up to you. Whatever you prefer, it's fine with me.
There was a problem hiding this comment.
I agree. I'm changing this to simply include a literal path in the expectation.
shadre
force-pushed
the
improve-log-output
branch
from
8b4109a to
ae9de7d
Compare
2 participants
No description provided.