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

testutil/compose: add definition version matrix test #833

Merged
merged 9 commits into from
Jul 22, 2022

Conversation

corverroos
Copy link
Contributor

Add a compose smoke test for backwards compatibility with v1.0.0 definition version.

Also enable run version matrix test again.

category: test
ticket: #710

@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #833 (0af79b8) into main (6e82ea9) will increase coverage by 0.53%.
The diff coverage is 30.76%.

@@            Coverage Diff             @@
##             main     #833      +/-   ##
==========================================
+ Coverage   54.78%   55.31%   +0.53%     
==========================================
  Files         111      111              
  Lines       11425    11534     +109     
==========================================
+ Hits         6259     6380     +121     
+ Misses       4259     4233      -26     
- Partials      907      921      +14     
Impacted Files Coverage Δ
p2p/sender.go 55.35% <0.00%> (ø)
testutil/compose/compose/alert.go 0.00% <0.00%> (ø)
testutil/compose/compose/main.go 0.00% <0.00%> (ø)
testutil/compose/config.go 83.33% <ø> (ø)
testutil/compose/template.go 43.75% <ø> (ø)
testutil/compose/run.go 84.21% <85.71%> (ø)
dkg/frostp2p.go 73.26% <100.00%> (+0.70%) ⬆️
testutil/compose/define.go 40.24% <100.00%> (ø)
testutil/compose/lock.go 70.87% <100.00%> (ø)
core/bcast/bcast.go 52.17% <0.00%> (-2.83%) ⬇️
... and 13 more

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 6e82ea9...0af79b8. Read the comment docs.

Comment on lines 92 to 96
RunTmplFunc: func(data *compose.TmplData) {
// Node 0 is latest
pegImageTag(data.Nodes, 1, "v8.0.0")
pegImageTag(data.Nodes, 2, "v7.0.0")
pegImageTag(data.Nodes, 3, "v6.0.0")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

re-enabled this test

Comment on lines 100 to 106
Name: "definition version matrix with dkg - v1.0.0",
ConfigFunc: func(conf *compose.Config) {
conf.KeyGen = compose.KeyGenDKG
},
DefineTmplFunc: func(data *compose.TmplData) {
// v8.0.0 of charon generates v1.0.0 defnition files.
pegImageTag(data.Nodes, 0, "v8.0.0")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added this test

@@ -147,11 +149,9 @@ func TestSmoke(t *testing.T) {
}
require.NoError(t, compose.WriteConfig(dir, conf))

cmd := newAutoCmd(func(data *compose.TmplData) {
data.MonitoringPorts = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pull this config up to compose API, see disable-monitoring field.

@@ -68,19 +68,17 @@ services:

prometheus:
image: prom/prometheus:latest
{{if .MonitoringPorts}}ports:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of not binding ports, rather just don't run monitoring at all (since not used if bound). Also saves some resources since less containers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, we do not monitoring since we rely on alerts for health checks

Copy link
Contributor

@xenowits xenowits left a comment

Choose a reason for hiding this comment

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

LGTM!

@corverroos corverroos added the merge when ready Indicates bulldozer bot may merge when all checks pass label Jul 22, 2022
@obol-bulldozer obol-bulldozer bot merged commit f679396 into main Jul 22, 2022
@obol-bulldozer obol-bulldozer bot deleted the corver/composeversion branch July 22, 2022 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants