Generalizing to the current directory to support all libraries, not just rails #157
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
schneems
reviewed
Dec 13, 2019
lib/derailed_benchmarks/tasks.rb
Outdated
| desc "runs the same test against two different branches for statistical comparison" | ||
| task :library do | ||
| task :library => [:app] do |
There was a problem hiding this comment.
You don't need to do this, the loading of the app is done when the sub script gets executed
|
There's a failure Looks like it's also failing on master https://travis-ci.org/schneems/derailed_benchmarks/builds/623900596?utm_medium=notification&utm_source=github_status It needs to be fixed, but it's not required before this gets merged in. We'll need a test that exercises that this task exists and is otherwise syntax error free. You can copy the We also need some docs in the README. |
|
Awesome! I think we're close.
That should just work ™️ unless there are some merge conflicts.
And put a "s" in front of each of your commits but the first one and this will "squash" your commits. Once you're happy, you can force push to your branch. |
ericc572
force-pushed
the
ericc572/generalize_to_other_libs
branch
from
c56dcab
to
5e3a6ef
Compare
|
Squashed commits and repushed. Odd that travis isn't showing up anymore though |
|
Not sure why it's failing here: https://travis-ci.org/schneems/derailed_benchmarks/jobs/626860575?utm_medium=notification&utm_source=github_status only for the builds where it's using the rails git gemfile. I see that if the In this test build, however, it will NOT skip since it is being set from that gemfile. Tried to handle this the same way as |
ericc572
force-pushed
the
ericc572/generalize_to_other_libs
branch
from
5e3a6ef
to
7a985f7
Compare
|
This should fix it: $ git diff HEAD^
diff --git a/test/integration/tasks_test.rb b/test/integration/tasks_test.rb
index 30a5a00..46c8617 100644
--- a/test/integration/tasks_test.rb
+++ b/test/integration/tasks_test.rb
@@ -13,6 +13,13 @@ class TasksTest < ActiveSupport::TestCase
FileUtils.remove_entry_secure(rails_app_path('tmp'))
end
+ def run!(cmd)
+ puts "Running: #{cmd}"
+ out = `#{cmd}`
+ raise "Could not run #{cmd}, output: #{out}" unless $?.success?
+ out
+ end
+
def rake(cmd, options = {})
assert_success = options.key?(:assert_success) ? options[:assert_success] : true
env = options[:env] || {}
@@ -57,8 +64,9 @@ class TasksTest < ActiveSupport::TestCase
test 'app' do
skip unless ENV['USING_RAILS_GIT']
+ run!("cd #{rails_app_path} && git init . && git add . && git commit -m first && git commit --allow-empty -m second")
- env = { "TEST_COUNT" => 10, "DERAILED_SCRIPT_COUNT" => 2, "SHAS_TO_TEST" => "3054e1d584e7eca110c69a1f8423f2e0866abbf9,80f989aecece1a2b1830e9c953e5887421b52d3c"}
+ env = { "TEST_COUNT" => 10, "DERAILED_SCRIPT_COUNT" => 2 }
puts rake "perf:app", { env: env }
endThe fixture app that is being used to run tests on is inside of a top-level git dir, so it does not have it's own git repo. This commit fakes this by creating a new git repo and two empty commits to run against. |
ericc572
force-pushed
the
ericc572/generalize_to_other_libs
branch
from
7a985f7
to
4d12f9c
Compare
…ust rails. Removing app dependency in library task Adding working smoke test Updating README Removing app dependency in library task Resolving merge conflict Adding working smoke test Updating README Adding changes to changelog Bringing in env change Updating README Generalizing to the current directory to support all libraries, not just rails. Removing app dependency in library task Resolving merge conflict Adding working smoke test Updating README Adding changes to changelog Bringing in env change
|
Great! I pushed up and it now passes the test :D |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, this only supports libraries if the user inputs the env variable. Now, we can take the current directory and run the performance tests comparing the two previous commits there.
CC: @schneems