Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/govim: add concept of user config in test overrides #606

Merged
merged 1 commit into from
Dec 8, 2019

Conversation

myitcv
Copy link
Member

@myitcv myitcv commented Dec 2, 2019

No description provided.

@myitcv myitcv force-pushed the cmd_govim_tests_user_config branch 4 times, most recently from 9020990 to bb54cf3 Compare December 5, 2019 14:47
@myitcv myitcv requested a review from leitzler December 5, 2019 16:14
Currently in our script tests we can specify the default config values
that will be used in the test. This overrides the cmd/govim default that
the end user will experience. Whilst this might be necessary in some
limited test situations, it's generally not what we neeed/intend. Rather
any config we're looking to supply in a scenario is generally an initial
override for the cmd/govim default.

Hence, we now all a test script to specify an explicit default via
default_config.json in a scenario directory (which would be nil, i.e. no
file), and we also allow for user_config.json to be the user's initial
overrides that are applied atop the defaults (whatever they may be).

As part of this we write a simple code generator to automate the
creation of the method:

  (*cmd/govim/config.Config).Apply(*cmd/govim/config.Config)

to avoid human mistakes further down the line.
@myitcv myitcv force-pushed the cmd_govim_tests_user_config branch from bb54cf3 to 153d7a9 Compare December 7, 2019 01:44
Copy link
Member

@leitzler leitzler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two questions, otherwise LGTM

@myitcv myitcv merged commit 30eab52 into master Dec 8, 2019
@myitcv myitcv deleted the cmd_govim_tests_user_config branch December 8, 2019 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants