LG-8399: IPP PO Search: Mock ArcGIS service generates fake but unique addresses#7698
Merged
allthesignals merged 9 commits intomainfrom Jan 30, 2023
Merged
LG-8399: IPP PO Search: Mock ArcGIS service generates fake but unique addresses#7698allthesignals merged 9 commits intomainfrom
allthesignals merged 9 commits intomainfrom
Conversation
…ker in PO Search developer mode
allthesignals
commented
Jan 25, 2023
| gem 'devise', '~> 4.8' | ||
| gem 'dotiw', '>= 4.0.1' | ||
| gem 'faraday', '~> 2' | ||
| gem 'faker' |
Contributor
Author
There was a problem hiding this comment.
Fixtures class, which are used for the development environment mock service, now leverages this gem, and so is moved out of the :test block.
aduth
approved these changes
Jan 26, 2023
| Rails.root.join('spec', 'fixtures', 'arcgis_responses', filename).read | ||
| end | ||
|
|
||
| private_class_method def self.generate_suggestions(count = 5) |
Contributor
There was a problem hiding this comment.
May be cleaner to move all the methods into class << self ?
i.e.
module ArcgisApi
module Mock
module Fixtures
class << self
def request_suggestions_response
# ...
end
# ...
private
def generate_suggestions(count = 5)
# ...
end
end
end
end
end
Contributor
There was a problem hiding this comment.
Alternatively, add a newline?
Suggested change
| private_class_method def self.generate_suggestions(count = 5) | |
| private_class_method | |
| def self.generate_suggestions(count = 5) |
Contributor
Author
There was a problem hiding this comment.
Hmm there is a mix of styles to doing this in the source code. I opted for:
private_class_method :generate_suggestions, :generate_address_candidates
🤷 not particularly sold on any one style.
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Contributor
Author
|
Some updates about this:
|
allthesignals
commented
Jan 30, 2023
Contributor
Author
There was a problem hiding this comment.
I wish I could move this above the methods (similar to Rails hooks), but it doesn't seem to know about the methods then.
6502ee3 to
bbb6335
Compare
Merged
This file contains hidden or 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
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.
🎫 Ticket
Link to the relevant ticket.
🛠 Summary of changes
Leverages Faker to get the ArcGIS mock service to return unique responses across requests. This reduces the severity of this bug as for as "DX" goes.
📜 Testing Plan
Provide a checklist of steps to confirm the changes.