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

bpf2go: argument parsing unit test #1054

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

mejedi
Copy link
Contributor

@mejedi mejedi commented Jun 7, 2023

Enable unit tests for argument parsing in bpf2go. This is in preparation for enabling bpf2go to take some arguments from the environment.

The patch moves argument parsing from run into a separate function.

}
}

b2g.strip, err = exec.LookPath(b2g.strip)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving LookupPath out of parseArgs so that unit tests can pass arbitrary values in -strip flag.

Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the quick turn around!

cmd/bpf2go/main.go Show resolved Hide resolved
makeBase string
}

func (b2g *bpf2go) parseArgs(args []string) (err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: my preference is to have constructor like function as much as possible. How would you feel about newB2G(stdout io.Writer, pkg, outputDir string, args []string) (*b2g, error)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mejedi mejedi force-pushed the bpf2go-option-parsing-test branch from 9a81965 to 5091cf1 Compare June 8, 2023 09:20
@mejedi mejedi requested a review from lmb June 8, 2023 09:21
}

input := args[1]
if _, err := os.Stat(input); os.IsNotExist(err) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving os.Stat out of newB2G so that unit tests can pass arbitrary values.

Signed-off-by: Nick Zavaritsky <mejedi@gmail.com>
@lmb lmb force-pushed the bpf2go-option-parsing-test branch from 5091cf1 to 3d8c466 Compare June 8, 2023 14:10
@lmb
Copy link
Collaborator

lmb commented Jun 8, 2023

Fixed a nit and rebased, thanks for refactoring this!

@mejedi
Copy link
Contributor Author

mejedi commented Jun 12, 2023

@lmb thank you for the reviews! Any chance this could be merged?

@lmb lmb merged commit 144ef91 into cilium:master Jun 12, 2023
@mejedi mejedi deleted the bpf2go-option-parsing-test branch June 12, 2023 09:46
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.

2 participants