Refactor tests to better reflect actual code usage #118
Conversation
| results_json = mock | ||
| results = mock(to_json: results_json) | ||
|
|
||
| File.expects(:read).with(SOLUTION_PATH / "two_fer.rb").returns(code) |
There was a problem hiding this comment.
This test is no longer useful because of the new method I added.
| protected | ||
| attr_reader :exercise_slug | ||
|
|
||
| def analysis_results(code = "") |
There was a problem hiding this comment.
I created a parent class with a method that is called by all of the existing tests. The new method creates a temporary file and then calls the actual AnalyzeSolution class to read and parse the file.
| code_to_analyze = File.read(solution_path / FILENAMES[exercise_slug]) | ||
|
|
||
| puts "Analysing #{exercise_slug}" | ||
| classified_exercise = exercise_slug.tr('-', '_').classify |
There was a problem hiding this comment.
While running the tests, I uncovered a bug where the 'classify' method was stripping the 's' off the end of 'HighScores' because the method was created to singularize Rails ActiveRecord table names into model names. This caused the analyzer to fail.
PatrickMcSweeny
force-pushed
the
analyzer-test
branch
from
8cc0861 to
db80b69
Compare
PatrickMcSweeny
force-pushed
the
analyzer-test
branch
2 times, most recently
from
2011cc7 to
a7a6bd7
Compare
PatrickMcSweeny
force-pushed
the
analyzer-test
branch
from
a7a6bd7 to
dbcc6de
Compare
|
@SleeplessByte Could you or someone else look at this? |
|
Does this still need guardian approval? Is it ready for a review? |
|
@IsaacG Yes, it is ready for review. |
|
@kotp Are you still working on/reviewing this PR? It would be good to have a review from @exercism/ruby |
Yes, but I do not have enough magic powder to activate approval here. So it does not really matter all that much that I approve, or just leave it and not approve. But I will definitely make noise if there is something I see that should not happen. ;) |
Your approval sends a strong signal to guardians that may not be very comfortable with Ruby! It also holds very high value as you are a track maintainer. Your approval may not directly allow the PR to be merged but it matters greatly. |
That is why I still take the time... :) |
4 participants
I previously mentioned creating this here.
After thinking about it and experimenting, I came to the conclusion that creating a temporary solution file that can be read by the analyzer was the best approach because it most accurately reflects how the analyzer is used in the real world.