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

Allow stages to be properly disabled by fixing the CLI parsing #786

Merged
merged 3 commits into from Oct 1, 2018

Conversation

na--
Copy link
Member

@na-- na-- commented Sep 28, 2018

This fixes #775 - now K6_STAGES="" k6 run script.js can properly overwrite stages defined in the script and k6 run --stage "" can overwrite stages configured by environment variables or from the script.

The len(conf.Stages) == 0 check covers both cases where stages are nil (i.e. weren't defined at all) and where they have been explicitly disabled from an upper configuration tier (empty slice).

I started to do some sort of robust end-to-end configuration testing framework for the future, but I postponed it for the future and just left a TODO since we'd have to refactor how we deal with the configuration a lot to be able to end-to-end test things properly... We also need to refactor it for a bunch of other reasons, so we'd do it then anyway.

@codecov-io
Copy link

codecov-io commented Sep 28, 2018

Codecov Report

Merging #786 into master will decrease coverage by 0.01%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #786      +/-   ##
==========================================
- Coverage   63.93%   63.92%   -0.02%     
==========================================
  Files         102      102              
  Lines        8496     8498       +2     
==========================================
  Hits         5432     5432              
- Misses       2702     2705       +3     
+ Partials      362      361       -1
Impacted Files Coverage Δ
cmd/options.go 60.18% <0%> (-2.08%) ⬇️
cmd/run.go 6.89% <0%> (ø) ⬆️
core/local/local.go 77.89% <100%> (ø) ⬆️
js/modules/k6/ws/ws.go 72.54% <0%> (+0.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef05dfd...1e8d720. Read the comment docs.

Copy link
Member

@robingustafsson robingustafsson left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -83,6 +83,8 @@ func TestConfigCmd(t *testing.T) {
}
}

//TODO: write end-to-end configuration tests - how the config file, in-script options, environment variables and CLI flags are parsed and interract... and how the end result is then propagated back into the script
Copy link
Member

Choose a reason for hiding this comment

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

"interract" => "interact"

@na-- na-- merged commit f6d0d2e into master Oct 1, 2018
@na-- na-- deleted the fix-stages-disabling branch October 1, 2018 05:39
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.

Cannot override exported stages with CLI flag or env. variable
3 participants