From 840768e52da444f7eb270e0f0070c60d3e84ac19 Mon Sep 17 00:00:00 2001 From: Ilya Yachin Date: Thu, 29 Oct 2020 22:39:58 +1000 Subject: [PATCH] fixes and enhancements for new release --- .xcccr.toml.example | 23 +++++--- README.md | 2 + RELEASENOTES.md | 12 +++- configurator.go | 37 ++++++++++++ reporter.go | 136 ++++++++++++++++++++++++-------------------- types.go | 29 ++++++---- 6 files changed, 155 insertions(+), 84 deletions(-) diff --git a/.xcccr.toml.example b/.xcccr.toml.example index f5fca3f..a7f9368 100644 --- a/.xcccr.toml.example +++ b/.xcccr.toml.example @@ -1,9 +1,14 @@ -ProjectPath = "/Users/home/" # Usualy ok to leave empty. If empty gets current working dir at runtime. -FilterTargets = ["CoreTests"] # If a target name contains one of these substrings the target would be filtered. Empty string "" matches everything. Please note that if regex is used targets filtering has no effect. -FilterPaths = ["Tests","UITests","Constants.swift"] # If a path contains one of these substrings the path (file, dir) would be filtered. Empty string "" matches everything. Please note that if regex is used path filtering has no effect. -FilterPattern = "[\\S\\s]*secret\\.json$" # Go flavored regex (non-PCRE!). Please check https://golang.org/pkg/regexp/ for more info. When used in config escape characters should be escaped twice for config parsing (unfortunately). Please note that if regex is used targets filtering has no effect. -InvertFilter = false # Set `false` to disallow targets and paths in filters. Set `true` to allow only targets and paths in filters. Default `false`. -IncludeMasked = false # Skip recalculating coverage, just read from xccov report regardless masked (filtered) targets and paths. Default `false`. -ZeroWarnOnly = false # Don't error on 0% coverage, produce warning only. -MeterLOC = false # Set `false` for coverage percent and `true` for number of LOCs e.g. 42/13 (now/was). Default `false`. -Tolerance = 5 # Percent. Default 0. \ No newline at end of file +ProjectPath = "/Users/home/" # Usualy ok to leave empty. If empty gets current working dir at runtime. +FilterTargets = ["CoreTests"] # If a target name contains one of these substrings the target would be filtered. Empty string "" matches everything. Please note that if regex is used targets filtering has no effect. +FilterPaths = ["Tests","UITests","Constants.swift"] # If a path contains one of these substrings the path (file, dir) would be filtered. Empty string "" matches everything. Please note that if regex is used path filtering has no effect. +FilterWarnPaths = ["Project/Target/File.swift"] # If a path contains one of these substrings warnings would be muted for the path (file, dir). Coverage for the paths would still be metered. +FilterPattern = "[\\S\\s]*secret\\.json$" # Go flavored regex (non-PCRE!). Please check https://golang.org/pkg/regexp/ for more info. When used in config escape characters should be escaped twice for config parsing (unfortunately). Please note that if regex is used `FilterTargets` and `FilterPaths` has no effect. +InvertFilter = false # Set `false` to allow toggling every filter individually. Set `true` to force `true` to all filters. Default `false`. +InvertTargetFilter = false # Set `false` to disallow targets in filter. Set `true` to allow only targets in filter. Default `false`. +InvertPathFilter = false # Set `false` to disallow paths in filter. Set `true` to allow only paths in filter. Default `false`. +InvertRegexp = false # Set `false` to disallow paths matching `FilterPattern`. Set `true` to allow only paths matching `FilterPattern`. Default `false`. +InvertWarnpathFilter = false # Set `false` to mute warnings for paths in filter. Set `true` to allow only warnings for paths in filter. Default `false`. +IncludeMasked = false # Skip recalculating coverage, just read from xccov report regardless masked (filtered) targets and paths. Default `false`. +ZeroWarnOnly = false # Don't error on 0% coverage, produce warning only. +MeterLOC = false # Set `false` for coverage percent and `true` for number of LOCs e.g. 42/13 (now/was). Default `false`. +Tolerance = 5 # Percent. Default 0. \ No newline at end of file diff --git a/README.md b/README.md index eeba389..fdc7092 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,10 @@ This dead simple tool only works with Apple's `xccov` generated code coverage re `-lst` | For path of "last report" file. A report to be base for determining whether the coverage metrics is lowered or became higher in "current" report. `-tol` | Percent to be tolerated before error. E.g. last is 70%, current is 65% and tol is 10 means no error. `-proj` | Project root to get relative paths. If empty and missing from config working dir would be used. Usually in GitHub Actions you just want the working dir. + `-nw` | Comma separated strings of paths not to warn. When a path has one of the values of this list warnings for the files on path would be muted. Coverage would still be metered. `-rg` | Regex filter, pass it in quotes e.g. `-rg="[\s\S]\.swift"`. Matching paths would be ignored. When used with `-i` the result is opposite: only matching paths would be visible to reporter. Please note that if regex is used targets and paths filtering set in config has no effect. `-i` | Invert filter. Applied to -rg and to targets/paths filter lists in config. If no filters supplied prevents reporting allowing nothing. + `-si` | Selectively invert filters. When `-i` is set it is equvalent to `-si=prtw` and always have precendence over this flag. Values here are: `p` — inverts paths filter, `r` — inverts regex, `t` — inverts targets filter, `w` — inverts warnings filter. `-m` | Include masked files coverage in total metrics. If true warnings would be produced only for unfiltered targets/paths, but total coverage would be reported for all the files as listed in original xccov json. `-loc` | Count covered LOCs diff instead of percent coverage. `-z` | Don't error on 0% coverage, produce warning only. diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 81e4445..f89f109 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,2 +1,10 @@ -- No breaking changes. New `ZeroWarnOnly` parameter added to config. Please check examples. -- This release still only tested manually. Help wanted to cover with tests. \ No newline at end of file +### Fixed +- File paths for warnings was always relative to working dir only. `ProjectPath` setting in config and `proj` flag was completely ignored. +- Removed some code duplication. +- Data race is cured by adding a wait group to wait for all file reports being processed. Sometimes looping through all the files was ended prior the last report is collected from channel leading to incomplete counts. No more. + +### Changed +- Now warnings for functions coverage aren't shown per line, but instead are included in file coverage warnings. It still includes xccov messages with the line numbers. +- New filter added to mute warnings for paths without excluding the path from coverage report. +- Filters may be inverted individually with new flags and config settings. Please check readme and config example for usage. +- No breaking changes were introduced. \ No newline at end of file diff --git a/configurator.go b/configurator.go index 070b7c0..9a72ff0 100644 --- a/configurator.go +++ b/configurator.go @@ -12,6 +12,8 @@ import ( "os" "regexp" "strconv" + "strings" + "sync" "github.com/BurntSushi/toml" ) @@ -46,6 +48,11 @@ func prepareRunConditions() (rnCnd *RunConditions, err error) { getWorkdir(), "Project root to get relative paths. If empty and missing from config working dir would be used.", ) + mute := flag.String( + "nw", + "", + "Comma separated strings of paths not to warn. When a path has one of the values of this list warnings for the files on path would be muted. Coverage would still be metered.", + ) filtr := flag.String("rg", dumbValue, "Regex filter. Applied to paths.", @@ -55,6 +62,11 @@ func prepareRunConditions() (rnCnd *RunConditions, err error) { false, "Invert filter. Applied to -rg and to targets/paths filter lists in config. If no filters supplied prevents reporting allowing nothing.", ) + selInvert := flag.String( + "si", + "", + "Selectively invert filters. When `-i` is set it is equvalent to `-si=prtw` and always have precendence over this flag. Values here are:\n`p` — inverts paths filter,\n`r` — inverts regex,\n`t` — inverts targets filter,\n`w` — inverts warnings filter.", + ) includeMasked := flag.Bool( "m", false, @@ -96,12 +108,35 @@ func prepareRunConditions() (rnCnd *RunConditions, err error) { } cfg.Tolerance = i } + if *mute != "" { + cfg.FilterWarnPaths = strings.Split(*mute, ",") + } if *filtr != dumbValue { cfg.FilterPattern = *filtr } if *fInvert { cfg.InvertFilter = true } + if cfg.InvertFilter { + cfg.InvertPathFilter = true + cfg.InvertRegexp = true + cfg.InvertTargetFilter = true + cfg.InvertWarnpathFilter = true + } + for _, c := range *selInvert { + if c == 'p' { + cfg.InvertPathFilter = true + } + if c == 'r' { + cfg.InvertRegexp = true + } + if c == 't' { + cfg.InvertTargetFilter = true + } + if c == 'w' { + cfg.InvertWarnpathFilter = true + } + } if *includeMasked { cfg.IncludeMasked = true } @@ -137,6 +172,8 @@ func prepareRunConditions() (rnCnd *RunConditions, err error) { } rnCnd.LastReport = lstXccRep + rnCnd.WaitGroup = &sync.WaitGroup{} + return } diff --git a/reporter.go b/reporter.go index 95b6f1d..0c2eccb 100644 --- a/reporter.go +++ b/reporter.go @@ -14,9 +14,10 @@ func performReport(rnCnd *RunConditions) { go func(tu *DiffUnit) { for { totalsUnit = <-rnCnd.TotalUnitChan + rnCnd.WaitGroup.Done() } }(&totalsUnit) - go unitCounter(rnCnd.DiffUnitChan, rnCnd.TotalUnitChan) + go unitCounter(rnCnd) lastFlatTgs := flattenTargets(last) for _, ct := range current.Targets { @@ -24,12 +25,13 @@ func performReport(rnCnd *RunConditions) { if lt, ok := lastFlatTgs[ct.Name]; ok { diffTargets(rnCnd, <, &ct) } else { - // inspectTargetFiles(rnCnd, &ct) diffTargets(rnCnd, &Target{}, &ct) } } } + rnCnd.WaitGroup.Wait() + produceVerdict(rnCnd.Config, &totalsUnit, last, current) } @@ -45,9 +47,9 @@ func produceVerdict(cfg *Config, tu *DiffUnit, last, current *XCCoverageReport) covMsg := "Code coverage is %s" if lastPct > currentPct { - covMsg = fmt.Sprintf(covMsg, "lowered") + covMsg = fmt.Sprintf(covMsg, "increased") } else if lastPct < currentPct { - covMsg = fmt.Sprintf(covMsg, "heightened") + covMsg = fmt.Sprintf(covMsg, "decreased") } else { covMsg = fmt.Sprintf(covMsg, "not changed") } @@ -88,68 +90,82 @@ func exitMessage(message string) { func diffTargets(rnCnd *RunConditions, lastTgt, currentTgt *Target) { ltgtFlatFiles := flattenFiles(lastTgt) + // Wait for every file coverage to be reported. + rnCnd.WaitGroup.Add(len(currentTgt.Files)) for _, curFile := range currentTgt.Files { - if pathIsAllowed(rnCnd, &curFile) { - if lstFile, ok := ltgtFlatFiles[curFile.Name]; ok { - rnCnd.DiffUnitChan <- DiffUnit{ - LastExecutableLines: lstFile.ExecutableLines, - LastCoveredLines: lstFile.CoveredLines, - CurrentExecutableLines: curFile.ExecutableLines, - CurrentCoveredLines: curFile.CoveredLines, - } - - lfsFlatFuncs := flattenFuncs(&lstFile) - for _, curFunc := range curFile.Functions { - if lstFunc, ok := lfsFlatFuncs[curFunc.Name]; ok { - warnFuncCov(curFile.Path, &lstFunc, &curFunc) - } else { - warnZeroFuncCov(curFile.Path, &curFunc) - } - } + if pathIsAllowed(rnCnd, &curFile) == false { + // Path is excluded from coverage report. + // Don't expect coverage unit. Try next. + rnCnd.WaitGroup.Done() + continue + } + + var lfsFlatFuncs map[string]Function + lstFile := ltgtFlatFiles[curFile.Name] + curFilePth := strings.TrimPrefix(curFile.Path, rnCnd.Config.ProjectPath) + + lfsFlatFuncs = flattenFuncs(&lstFile) + rnCnd.DiffUnitChan <- DiffUnit{ + LastExecutableLines: lstFile.ExecutableLines, + LastCoveredLines: lstFile.CoveredLines, + CurrentExecutableLines: curFile.ExecutableLines, + CurrentCoveredLines: curFile.CoveredLines, + } + + if warnIsAllowed(rnCnd, &curFile) == false { + // Don't warn, count coverage only. + continue + } + + fileReport := "\n" + if curFile.CoveredLines < lstFile.CoveredLines { + var warnMsg string + if rnCnd.Config.MeterLOC { + warnMsg = fmt.Sprintf("covered %dLOC, was %dLOC.", curFile.CoveredLines, lstFile.CoveredLines) + } + fileReport = fmt.Sprintf("File coverage is reduced: %s\n", warnMsg) + } + if curFile.CoveredLines == 0 { + fileReport = "File is not covered.\n" + } + + for _, curFunc := range curFile.Functions { + if lstFunc, ok := lfsFlatFuncs[curFunc.Name]; ok { + fileReport = fmt.Sprintf("%s%s", fileReport, warnFuncCov(&lstFunc, &curFunc)) } else { - rnCnd.DiffUnitChan <- DiffUnit{ - LastExecutableLines: 0, - LastCoveredLines: 0, - CurrentExecutableLines: curFile.ExecutableLines, - CurrentCoveredLines: curFile.CoveredLines, - } - - inspectFileFuncs(&curFile) + fileReport = fmt.Sprintf("%s%s", fileReport, warnZeroFuncCov(&curFunc)) } } - } -} + if fileReport != "\n" { + fmt.Printf("::warning file=%s::%s", curFilePth, fileReport) + } -func inspectFileFuncs(file *File) { - for _, ff := range file.Functions { - warnZeroFuncCov(file.Path, &ff) } } -func warnFuncCov(filePth string, lstFunc, curFunc *Function) { +func warnFuncCov(lstFunc, curFunc *Function) string { if lstFunc.LineCoverage > curFunc.LineCoverage { - fmt.Printf( - "::warning file=%s,line=%d::%s coverage is lowered to %d%% (was %d%%).\n", - strings.TrimPrefix(filePth, getWorkdir()), + return fmt.Sprintf( + "%d | %s coverage is lowered to %d%% (was %d%%).\n", curFunc.LineNumber, curFunc.Name, percentify(curFunc.LineCoverage), percentify(lstFunc.LineCoverage), ) - return + } - warnZeroFuncCov(filePth, curFunc) + return warnZeroFuncCov(curFunc) } -func warnZeroFuncCov(filePth string, curFunc *Function) { +func warnZeroFuncCov(curFunc *Function) string { if percentify(curFunc.LineCoverage) == 0 { - fmt.Printf( - "::warning file=%s,line=%d::%s coverage is 0%%.\n", - strings.TrimPrefix(filePth, getWorkdir()), + return fmt.Sprintf( + "%d | %s is not covered.\n", curFunc.LineNumber, curFunc.Name, ) } + return "" } func linesRatio(cov, exec int) float64 { @@ -191,23 +207,18 @@ func targetIsAllowed(rnCnd *RunConditions, tgt *Target) bool { if rnCnd.Regexp != nil { return true // Allow any target and let regex sortout filtering on paths. } - return elInList(rnCnd.Config.FilterTargets, tgt.Name) == rnCnd.Config.InvertFilter + return elSubInList(rnCnd.Config.FilterTargets, tgt.Name) == rnCnd.Config.InvertTargetFilter } func pathIsAllowed(rnCnd *RunConditions, fl *File) bool { if rnCnd.Regexp != nil { return elemAllowedWithRegex(rnCnd, fl.Path) } - return elSubInList(rnCnd.Config.FilterPaths, fl.Path) == rnCnd.Config.InvertFilter + return elSubInList(rnCnd.Config.FilterPaths, fl.Path) == rnCnd.Config.InvertPathFilter } -func elInList(elList []string, elName string) bool { - for _, el := range elList { - if strings.Contains(elName, el) { - return true - } - } - return false +func warnIsAllowed(rnCnd *RunConditions, fl *File) bool { + return elSubInList(rnCnd.Config.FilterWarnPaths, fl.Path) == rnCnd.Config.InvertWarnpathFilter } func elSubInList(elList []string, elName string) bool { @@ -223,19 +234,18 @@ func elemAllowedWithRegex(rnCnd *RunConditions, el string) bool { if rnCnd.Regexp == nil { return true } - return (rnCnd.Regexp.Find([]byte(el)) != nil) == rnCnd.Config.InvertFilter + return (rnCnd.Regexp.Find([]byte(el)) != nil) == rnCnd.Config.InvertRegexp } -func unitCounter(duChan, totalChan chan DiffUnit) { +func unitCounter(rnCnd *RunConditions) { totalDu := DiffUnit{} for { - select { - case u := <-duChan: - totalDu.CurrentCoveredLines += u.CurrentCoveredLines - totalDu.CurrentExecutableLines += u.CurrentExecutableLines - totalDu.LastCoveredLines += u.LastCoveredLines - totalDu.LastExecutableLines += u.LastExecutableLines - case totalChan <- totalDu: - } + u := <-rnCnd.DiffUnitChan + totalDu.CurrentCoveredLines += u.CurrentCoveredLines + totalDu.CurrentExecutableLines += u.CurrentExecutableLines + totalDu.LastCoveredLines += u.LastCoveredLines + totalDu.LastExecutableLines += u.LastExecutableLines + + rnCnd.TotalUnitChan <- totalDu } } diff --git a/types.go b/types.go index 1fda32e..a1d3804 100644 --- a/types.go +++ b/types.go @@ -1,6 +1,9 @@ package main -import "regexp" +import ( + "regexp" + "sync" +) // Report type XCCoverageReport struct { @@ -36,15 +39,20 @@ type Target struct { // App config type Config struct { - ProjectPath string - FilterTargets []string - FilterPaths []string - FilterPattern string - InvertFilter bool - IncludeMasked bool - ZeroWarnOnly bool - MeterLOC bool - Tolerance int + ProjectPath string + FilterTargets []string + FilterPaths []string + FilterWarnPaths []string + FilterPattern string + InvertFilter bool + InvertTargetFilter bool + InvertPathFilter bool + InvertRegexp bool + InvertWarnpathFilter bool + IncludeMasked bool + ZeroWarnOnly bool + MeterLOC bool + Tolerance int } type RunConditions struct { LastReport *XCCoverageReport @@ -53,6 +61,7 @@ type RunConditions struct { Regexp *regexp.Regexp DiffUnitChan chan DiffUnit TotalUnitChan chan DiffUnit + WaitGroup *sync.WaitGroup } // Coverage diff unit