Feature/missing uploaders #81
Conversation
|
|
||
| legislation = Legislation.find_by(title: 'Climate Law') | ||
|
|
||
| expect(legislation.legislation_type).to eq('legislative') |
There was a problem hiding this comment.
maybe expect(legislation).to have_attributes(..)?
| legislation2 = create(:legislation) | ||
| updated_litigation = create(:litigation) | ||
|
|
||
| csv_content = <<-CSV |
There was a problem hiding this comment.
I understand that this needs to be dynamic here, but can we still keep this in let-vars?
There was a problem hiding this comment.
I will see if that will be more readable, maybe if we split those tests to separate files to test only importer services.
There was a problem hiding this comment.
yeah, maybe we can move to lets just things like legislation2 , updated_litigation?
There was a problem hiding this comment.
Hmm, moving those values to let globally would make test less readable in this case. Everything declared directly in test makes things easier when the test file is too big. There should be either context or separate file, but I don't think we have time to refactor tests much.
There was a problem hiding this comment.
Lol, I should have picked MiniTest instead of RSpec, fewer things to remember. Minimalism FTW!
|
|
||
| litigation = Litigation.find_by(title: 'Litigation number 1') | ||
|
|
||
| expect(litigation.jurisdiction.iso).to eq('GBR') |
There was a problem hiding this comment.
same here: have_attributes() matcher could be more readable.
There was a problem hiding this comment.
hmm, have_attributes nice idea 👍
| file_path = "#{Rails.root}/spec/support/fixtures/files/#{filename}" | ||
|
|
||
| if content.present? | ||
| file_path = "#{Rails.root}/tmp/litigation_sides.csv" |
There was a problem hiding this comment.
hmm, why we have reference to litigation_sides in generic fixture_file helper?:)
There was a problem hiding this comment.
oh shit, good catch. That file name for tmp file does not actually matter but should be tmp/#{filename}
I was planning to get rid of AWS in this PR, but I will do that tomorrow in a separate one.
Couple things here: