Conversation
3v0k4
force-pushed
the
fallback-error
branch
2 times, most recently
from
5ae80e4 to
8e1e627
Compare
3v0k4
force-pushed
the
fallback-error
branch
from
8e1e627 to
f9b3fde
Compare
|
Ensure the new feature works in a real project scenario just in case. # .circleci/config.yml
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# fallback ||
export KNAPSACK_PRO_ENDPOINT=https://api-fake.knapsackpro.com
export KNAPSACK_PRO_MAX_REQUEST_RETRIES=1
bundle exec rake knapsack_pro:rspec
# add the following
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# fallback off and custom exit code ||
export KNAPSACK_PRO_ENDPOINT=https://api-fake.knapsackpro.com
export KNAPSACK_PRO_FALLBACK_MODE_ENABLED=false
export KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE=0 # won't fail the build and it won't run tests
bundle exec rake knapsack_pro:rspec
...
# queue
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# fallback ||
export KNAPSACK_PRO_ENDPOINT=https://api-fake.knapsackpro.com
export KNAPSACK_PRO_MAX_REQUEST_RETRIES=1
bundle exec rake knapsack_pro:queue:rspec
# add the following
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# fallback off and custom exit code ||
export KNAPSACK_PRO_ENDPOINT=https://api-fake.knapsackpro.com
export KNAPSACK_PRO_FALLBACK_MODE_ENABLED=false
export KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE=0 # won't fail the build and it won't run tests
bundle exec rake knapsack_pro:queue:rspec
|
| end | ||
|
|
||
| def fallback_mode_error_exit_code_or(default) | ||
| env_value = ENV['KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE'] |
There was a problem hiding this comment.
suggestion
I would do this:
| env_value = ENV['KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE'] | |
| ENV.fetch('KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE', 1).to_i |
I would add a unit test in spec/knapsack_pro/config/env_spec.rb for KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE to explain there that 1 means the default exit code when KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE is not defined.
I would rename fallback_mode_error_exit_code_or to fallback_mode_error_exit_code.
No need to define 1 as default exit code each time you call this method in multiple test runners. By default 1 is exit code in case of an error.
There was a problem hiding this comment.
I would add a unit test in spec/knapsack_pro/config/env_spec.rb for KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE to explain there that 1 means the default exit code when KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE is not defined.
I think it's clear enough by looking at the code, no need to add test cases.
I would rename fallback_mode_error_exit_code_or to fallback_mode_error_exit_code.
No need to define 1 as default exit code each time you call this method in multiple test runners. By default 1 is exit code in case of an error.
Great point. Applied.
I decided not to test it e2e because:
I'd prefer to keep the e2e tests as smoke tests that don't cover edge cases. |
2 participants
Story
https://trello.com/c/GCRO05YP/513-custom-exit-code-when-cannot-connect-to-the-api-and-fallback-mode-is-disabled
Description
Allow users to set a custom error exit code (
KNAPSACK_PRO_FALLBACK_MODE_ERROR_EXIT_CODE) whenever Knapsack Pro fails because Fallback Mode cannot be used.Since we are catching the error, we don't print the backtraces anymore. This is a plus as you'd expect those only in case of an unhandled error. See below for the actual differences.
Checklist reminder
UNRELEASEDsection of theCHANGELOG.md, including the needed bump (ie, patch, minor, major)lib/knapsack_pro/pure/queue/rspec_pure.rbcontains pure functions that are unit tested.lib/knapsack_pro/extensions/rspec_extension.rbencapsulates calls to RSpec internals and is integration and e2e tested.lib/knapsack_pro/runners/queue/rspec_runner.rbinvokes the pure code and the extension to produce side effects, which are integration and e2e tested.Comparison before/after
I run locally Knapsack Pro with:
And additionally (to check the other failure path):
bin/knapsack_pro_rspec; echo $?after
before
bin/knapsack_pro_queue_rspec; echo $?after
It's a bit unfortunate that we print the summary as it seems nothing went wrong. But I think it's not that bad.
before