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.

Sign up for GitHub

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

Jump to bottom

Add include package to x-pack Metricbeat #9509

Merged
merged 1 commit into from
Dec 13, 2018
Merged

Add include package to x-pack Metricbeat #9509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions x-pack/metricbeat/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ package cmd
import (
"github.com/elastic/beats/metricbeat/cmd"
xpackcmd "github.com/elastic/beats/x-pack/libbeat/cmd"

// Register the includes.
_ "github.com/elastic/beats/x-pack/metricbeat/include"
ruflin marked this conversation as resolved.
Show resolved Hide resolved
)

// RootCmd to handle beats cli
Expand Down
27 changes: 27 additions & 0 deletions x-pack/metricbeat/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
Copy link
Member Author

Choose a reason for hiding this comment

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

@andrewkroh To build this binary should we add the logic to the Makefile or try to do it also in mage? Do we have that already somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

@andrewkroh I just realised in 6.x filebeat x-pack I can run make filebeat.test but not in 7.0. I wonder if that is a missing PR in master or was removed on purpose?

Copy link
Member

Choose a reason for hiding this comment

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

It was removed on purpose. The Makefiles in x-pack// are really minimal with just enough targets to make the existing CI systems continue to work. When you execute any of the targets that require the binary it will be produced (like mage unitTest, mage integTest, mage pythonUnitTest, or mage pythonIntegTest).

beats/x-pack/metricbeat/magefile.go

Line 155 in 6a4ce43

mg.Deps(mage.BuildSystemTestBinary)

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about having a mage target for just building the test binary? I use this pretty often for manual test runs if I only want to run a subset of system tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I won't add another command to the extensive list of commands we already have / need to use before every push to repo. Instead, I'll integrate this action (say build) within each command which interacts directly with the Go code (almost all commands now I guess) as the first step in a chain of actions, for example before setting up compose or checking fields: you don't compile so I fail in 20s and my CI slot is free again. Go compile time is pretty fast and this way we reduce cognitive load to everybody.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I can follow. I'm not worried about the failing of the building the Beats binary but how I run for example the system tests only for the Elasticsearch module. To do this, I need to test binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, system tests should be run with mage too, so mage should check compile correctness before running system tests but not as a separate command. I propose that compile correctness is integrated within the system test command (and unit test, integration test, etc.) so it's checked always.

Copy link
Member Author

Choose a reason for hiding this comment

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

I still don't understand how I will run a single test locally if we don't have the target without me manually coming up with the details for building it each time? My goal here is not to figure out if the binary compiles or not as part of CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

So yeah, my goal is that it's done within the CI :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's assume I build module foo localy: How do I test only this module locally? And I do not want to test the binary but if the module works. Pushing always to CI to see if it works is not an option as I want to be able to develop offline and that would be way too slow for quick development feedback.

// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.
package main

// This file is mandatory as otherwise the metricbeat.test binary is not generated correctly.
import (
"flag"
"testing"

"github.com/elastic/beats/metricbeat/cmd"
)

var systemTest *bool

func init() {
systemTest = flag.Bool("systemTest", false, "Set to true when running system tests")
cmd.RootCmd.PersistentFlags().AddGoFlag(flag.CommandLine.Lookup("systemTest"))
cmd.RootCmd.PersistentFlags().AddGoFlag(flag.CommandLine.Lookup("test.coverprofile"))
}

// Test started when the test binary is started. Only calls main.
func TestSystem(t *testing.T) {
if *systemTest {
main()
}
}