WIP spec helper improvement #267
Conversation
|
|
||
| module ClientHelper | ||
| class << self | ||
| def sample_shapes |
There was a problem hiding this comment.
I think this model should work the same way our other smithy models do - we should have a model.smithy + model.json and we load that here.
Its terrible to work in smithy json. Its even worse to work in ruby hashes that are trying to look like json.
There was a problem hiding this comment.
The advantage to having the model json in code, prior to generation, is that we can modify contents in the test. For example v3 code has tests like:
shapes['StructureShape']['members']['String']['locationName'] = 'str'
expect(json(string: 'abc')).to eq('{"str":"abc"}')
There was a problem hiding this comment.
Yeah I like that ability - I guess I'm not suggesting that we can't have that - just that where the default comes from is a smithy model rather than an in code hash which is just kinda ugly and super annoying to edit.
There was a problem hiding this comment.
Ideally this is only written once, but yeah we can have a fixture default I suppose, but we need a way to modify it at runtime I think.
| $LOAD_PATH.delete("#{tmpdir}/lib") | ||
| end | ||
|
|
||
| def source(type, options = {}) |
There was a problem hiding this comment.
I find source a slightly weird name for this since we are doing the eval on it and not returning source.
There was a problem hiding this comment.
Yes, it's really just a sourced client. I guess this could be "from_source" and "from_files"
| Object.module_eval(source) | ||
| Object.const_get(plan.module_name) | ||
| [plan.module_name] | ||
| rescue Exception => e # rubocop:disable Lint/RescueException |
There was a problem hiding this comment.
We may actually want to divide the error handling into generating code and evaluating that code.
| tmpdir = plan.destination_root | ||
| Smithy.generate(plan) | ||
| $LOAD_PATH << ("#{tmpdir}/lib") | ||
| require plan.gem_name |
There was a problem hiding this comment.
In theory maybe we should also clean this up off the load path?
There was a problem hiding this comment.
That is done already in cleanup_generated
| @@ -43,7 +43,7 @@ def post_process(artifacts) | |||
| # rubocop:enable Lint/UselessAssignment | |||
|
|
|||
| after(:all) do | |||
| SpecHelper.cleanup(['Weather'], @tmpdir) | |||
| SpecHelper.cleanup_generated(['Weather'], @tmpdir) | |||
There was a problem hiding this comment.
This isn't directly related to this rectoring, but is something I've been thinking about - rather than needing to do this hacky before(:all)/after(:all) - we could have a with_generated method that would setup a before and after block for us.
There was a problem hiding this comment.
I think shared context might work for that, but I don't like wrapping all of the tests in yet another block. I think it would be nice to collect all of the generated clients and handle them at clean up automatically, perhaps by having state in the helper.
Refactor spec helper to understand how to generate clients, types, servers, and use a client helper which uses dynamic or static code generation.