Skip to content

Conversation

@Neal910
Copy link

@Neal910 Neal910 commented Oct 30, 2020

changes

This adds a new feature. You can set env var KNAPSACK_PRO_TEST_FILE_LIST_SOURCE_FILE to a path to the source file with a list of test files to be run. The source file should contain test files in a new line each.

More can be found in the README that was updated in this PR.
https://github.com/KnapsackPro/knapsack_pro-ruby#how-to-run-a-specific-list-of-test-files-or-only-some-tests-from-test-file

related issue

to resolve the issue #128

@ArturT
Copy link
Member

ArturT commented Oct 30, 2020

@Neal910 Can you give me access to push to your branch on the forked repo https://github.blog/2016-09-07-improving-collaboration-with-forks/

Copy link
Member

@ArturT ArturT left a comment

Choose a reason for hiding this comment

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

It would be also good to add info in README about this new feature KNAPSACK_PRO_TEST_FILE_LIST_SOURCE_FILE and how it works.

@ArturT
Copy link
Member

ArturT commented Nov 3, 2020

@Neal910 Will you be able to apply improvements from my comments? If not then I can handle it in the upcoming days. I could add this to my TODO list (I already got access to your repo so I could push commits with improvements).

@Neal910 Neal910 closed this Nov 3, 2020
@Neal910 Neal910 reopened this Nov 3, 2020
@Neal910 Neal910 requested a review from ArturT November 3, 2020 20:33
@ArturT ArturT changed the title allow accept file containing test files Allow accepting file containing test files to run - add support for env var KNAPSACK_PRO_TEST_FILE_LIST_SOURCE_FILE Nov 3, 2020
@ArturT
Copy link
Member

ArturT commented Nov 3, 2020

I've updated PR title and description so when someone visits this PR from changelog file it will be easier to understand what's the new feature.

@Neal910 Could you update the CHANGELOG.md file and add 2.7.0 version header with link to this PR. Thanks.

@ArturT ArturT self-requested a review November 3, 2020 22:26
Feng Sha and others added 7 commits November 3, 2020 17:28
Add more explanation why we added KNAPSACK_PRO_TEST_FILE_LIST_SOURCE_FILE and mark it with `KNAPSACK_PRO_TEST_FILE_LIST_SOURCE_FILE`
add new lines
@ArturT
Copy link
Member

ArturT commented Nov 3, 2020

@Neal910 Can I merge this PR now? If not then I can check it tomorrow morning.

PR looks ok to me. Great work @Neal910 🚀

@Neal910
Copy link
Author

Neal910 commented Nov 4, 2020

Sure, let 's merge it. Thanks a lot for your quick response.

@ArturT
Copy link
Member

ArturT commented Nov 4, 2020

Related PR in crystalball gem repo pluff/crystalball#129 for issue pluff/crystalball#117
(adding link here to future reference)

@Neal910
Copy link
Author

Neal910 commented Nov 4, 2020

It's weird, when I test it with my real data, I always got a comma added in each of the test file as following:

D, [2020-11-04T16:12:03.234132 #55150] DEBUG -- : [knapsack_pro] POST https://api.knapsackpro.com/v1/queues/queue
D, [2020-11-04T16:12:03.234195 #55150] DEBUG -- : [knapsack_pro] API request UUID: ******
D, [2020-11-04T16:12:03.234223 #55150] DEBUG -- : [knapsack_pro] API response:
D, [2020-11-04T16:12:03.234280 #55150] DEBUG -- : [knapsack_pro] {"queue_name"=>"1664:80de9629aa5c79bbdd93a749530f5824", "build_subset_id"=>nil, "test_files"=>[{"path"=>"spec/models/******_spec.rb[1],", "time_execution"=>nil}]}
I, [2020-11-04T16:12:03.234424 #55150]  INFO -- : [knapsack_pro] To retry in development the subset of tests fetched from API queue please run below command on your machine. If you use --order random then remember to add proper --seed 123 that you will find at the end of rspec command.
I, [2020-11-04T16:12:03.234506 #55150]  INFO -- : [knapsack_pro] bundle exec rspec --format documentation --format RspecJunitFormatter --out tmp/rspec.xml  --default-path spec "spec/models/******_spec.rb[1],"

An error occurred while loading ./spec/models/******_spec.rb[1],.
Failure/Error: return load_without_bootsnap(resolved, wrap)

LoadError:
  cannot load such file -- /Users/******/Documents/dev/******/spec/models/******_spec.rb[1],
# ./Rakefile:33:in `execute_with_timestamps'
No examples found.

I am sure that in my source file, it doesn't have the comma. Not sure where it's been added.
@ArturT

@ArturT
Copy link
Member

ArturT commented Nov 4, 2020

@Neal910 I tested this feature on my rails app and it works fine.
https://github.com/KnapsackPro/rails-app-with-knapsack_pro/pull/25/files

Are you sure your source file has no commas?

@Neal910
Copy link
Author

Neal910 commented Nov 4, 2020

oh, I think something is really wrong in my env, this is not the issue of KnapsackPro. Sorry to bother you.

@ArturT
Copy link
Member

ArturT commented Nov 4, 2020

@Neal910 If you need to debug the source code of knapsack_pro gem you can add in Gemfile:

if you debug on CI

# Gemfile
gem 'knapsack_pro', github: 'KnapsackPro/knapsack_pro-ruby', branch: ‘master’

and you can provide your own branch and forked repo name for github key then you can tweak code on your branch. You could try to add puts to see what data is parsed from the source file with list of test files.

if you debug in development

Gemfile:

# provide path to copy of knapsack_pro repo on your disk
gem 'knapsack_pro', path: '../knapsack_pro-ruby'

Then you can plug into the source code of knapsack_pro with pry gem https://github.com/pry/pry
Add the below line in any of the source files of forked knapsack_pro gem on your local disk to debug it:
require 'pry'; binding.pry

@Neal910 Neal910 deleted the accept-rspec-list-file branch November 5, 2020 02:48
@Neal910
Copy link
Author

Neal910 commented Nov 5, 2020

Tested on our CI, all good. Thanks for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants