Simplify coverage setting and comply to some requests in code review
This commit is contained in:
parent
bb12ee051f
commit
f1e920ed86
6 changed files with 29 additions and 63 deletions
|
@ -286,24 +286,11 @@ build outputs. Setting this up globally will make all the jobs to use this
|
|||
setting for output filtering and extracting the coverage information from your
|
||||
builds.
|
||||
|
||||
#### coverage:output_filter
|
||||
|
||||
For now, there is only the `output_filter` directive expected to be inside the
|
||||
`coverage` entry. And it is expected to be a regular expression.
|
||||
|
||||
So, in the end, you're going to have something like the following:
|
||||
Regular expressions are used by default. So using surrounding `/` is optional, given it'll always be read as a regular expression. Don't forget to escape special characters whenever you want to match them in the regular expression.
|
||||
|
||||
A simple example:
|
||||
```yaml
|
||||
coverage:
|
||||
output_filter: /\(\d+\.\d+\) covered\./
|
||||
```
|
||||
|
||||
It's worth to keep in mind that the surrounding `/` is optional. So, the above
|
||||
example is the same as the following:
|
||||
|
||||
```yaml
|
||||
coverage:
|
||||
output_filter: \(\d+\.\d+\) covered\.
|
||||
coverage: \(\d+\.\d+\) covered\.
|
||||
```
|
||||
|
||||
## Jobs
|
||||
|
@ -347,7 +334,6 @@ job_name:
|
|||
| before_script | no | Override a set of commands that are executed before build |
|
||||
| after_script | no | Override a set of commands that are executed after build |
|
||||
| environment | no | Defines a name of environment to which deployment is done by this build |
|
||||
| environment | no | Defines a name of environment to which deployment is done by this build |
|
||||
| coverage | no | Define coverage settings for a given job |
|
||||
|
||||
### script
|
||||
|
@ -1032,17 +1018,15 @@ been defined in the global level. A quick example of one overwritting the
|
|||
other would be:
|
||||
|
||||
```yaml
|
||||
coverage:
|
||||
output_filter: /\(\d+\.\d+\) covered\./
|
||||
coverage: \(\d+\.\d+\) covered\.
|
||||
|
||||
job1:
|
||||
coverage:
|
||||
output_filter: /Code coverage: \d+\.\d+/
|
||||
coverage: Code coverage: \d+\.\d+
|
||||
```
|
||||
|
||||
In the example above, considering the context of the job `job1`, the coverage
|
||||
regex that would be used is `/Code coverage: \d+\.\d+/` instead of
|
||||
`/\(\d+\.\d+\) covered\./`.
|
||||
regex that would be used is `Code coverage: \d+\.\d+` instead of
|
||||
`\(\d+\.\d+\) covered\.`.
|
||||
|
||||
## Git Strategy
|
||||
|
||||
|
|
|
@ -61,7 +61,7 @@ module Ci
|
|||
allow_failure: job[:allow_failure] || false,
|
||||
when: job[:when] || 'on_success',
|
||||
environment: job[:environment_name],
|
||||
coverage_regex: job[:coverage][:output_filter],
|
||||
coverage_regex: job[:coverage],
|
||||
yaml_variables: yaml_variables(name),
|
||||
options: {
|
||||
image: job[:image],
|
||||
|
|
|
@ -8,26 +8,16 @@ module Gitlab
|
|||
class Coverage < Node
|
||||
include Validatable
|
||||
|
||||
ALLOWED_KEYS = %i[output_filter]
|
||||
|
||||
validations do
|
||||
validates :config, type: Hash
|
||||
validates :config, allowed_keys: ALLOWED_KEYS
|
||||
validates :output_filter, regexp: true
|
||||
end
|
||||
|
||||
def output_filter
|
||||
output_filter_value = @config[:output_filter].to_s
|
||||
|
||||
if output_filter_value.start_with?('/') && output_filter_value.end_with?('/')
|
||||
output_filter_value[1...-1]
|
||||
else
|
||||
@config[:output_filter]
|
||||
end
|
||||
validates :config, regexp: true
|
||||
end
|
||||
|
||||
def value
|
||||
@config.merge(output_filter: output_filter)
|
||||
if @config.start_with?('/') && @config.end_with?('/')
|
||||
@config[1...-1]
|
||||
else
|
||||
@config
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,26 +9,17 @@ module Ci
|
|||
subject { described_class.new(config, path).build_attributes(:rspec) }
|
||||
|
||||
let(:config_base) { { rspec: { script: "rspec" } } }
|
||||
let(:config) { YAML.dump(config_base) }
|
||||
let(:config) { YAML.dump(config_base) }
|
||||
|
||||
context 'when config has coverage set at the global scope' do
|
||||
before do
|
||||
config_base.update(
|
||||
coverage: { output_filter: '\(\d+\.\d+\) covered' }
|
||||
)
|
||||
end
|
||||
before { config_base.update(coverage: '\(\d+\.\d+\) covered') }
|
||||
|
||||
context 'and \'rspec\' job doesn\'t have coverage set' do
|
||||
context "and 'rspec' job doesn't have coverage set" do
|
||||
it { is_expected.to include(coverage_regex: '\(\d+\.\d+\) covered') }
|
||||
end
|
||||
|
||||
context 'but \'rspec\' job also has coverage set' do
|
||||
before do
|
||||
config_base[:rspec].update(
|
||||
coverage: { output_filter: '/Code coverage: \d+\.\d+/' }
|
||||
)
|
||||
end
|
||||
|
||||
before { config_base[:rspec][:coverage] = '/Code coverage: \d+\.\d+/' }
|
||||
it { is_expected.to include(coverage_regex: 'Code coverage: \d+\.\d+') }
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5,35 +5,35 @@ describe Gitlab::Ci::Config::Entry::Coverage do
|
|||
|
||||
describe 'validations' do
|
||||
context 'when entry config value is correct' do
|
||||
let(:config) { { output_filter: 'Code coverage: \d+\.\d+' } }
|
||||
let(:config) { 'Code coverage: \d+\.\d+' }
|
||||
|
||||
describe '#value' do
|
||||
subject { entry.value }
|
||||
it { is_expected.to eq config }
|
||||
it { is_expected.to eq config }
|
||||
end
|
||||
|
||||
describe '#errors' do
|
||||
subject { entry.errors }
|
||||
it { is_expected.to be_empty }
|
||||
it { is_expected.to be_empty }
|
||||
end
|
||||
|
||||
describe '#valid?' do
|
||||
subject { entry }
|
||||
it { is_expected.to be_valid }
|
||||
it { is_expected.to be_valid }
|
||||
end
|
||||
end
|
||||
|
||||
context 'when entry value is not correct' do
|
||||
let(:config) { { output_filter: '(malformed regexp' } }
|
||||
let(:config) { '(malformed regexp' }
|
||||
|
||||
describe '#errors' do
|
||||
subject { entry.errors }
|
||||
it { is_expected.to include /coverage output filter must be a regular expression/ }
|
||||
it { is_expected.to include /coverage config must be a regular expression/ }
|
||||
end
|
||||
|
||||
describe '#valid?' do
|
||||
subject { entry }
|
||||
it { is_expected.not_to be_valid }
|
||||
it { is_expected.not_to be_valid }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -222,9 +222,10 @@ describe Ci::Build, :models do
|
|||
end
|
||||
|
||||
describe '#coverage_regex' do
|
||||
subject { build.coverage_regex }
|
||||
subject { build.coverage_regex }
|
||||
|
||||
let(:project_regex) { '\(\d+\.\d+\) covered' }
|
||||
let(:build_regex) { 'Code coverage: \d+\.\d+' }
|
||||
let(:build_regex) { 'Code coverage: \d+\.\d+' }
|
||||
|
||||
context 'when project has build_coverage_regex set' do
|
||||
before { project.build_coverage_regex = project_regex }
|
||||
|
@ -235,7 +236,7 @@ describe Ci::Build, :models do
|
|||
|
||||
context 'but coverage_regex attribute is also set' do
|
||||
before { build.coverage_regex = build_regex }
|
||||
it { is_expected.to eq(build_regex) }
|
||||
it { is_expected.to eq(build_regex) }
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue