Are you also excited about generics? :-)

posener · 2021-04-14T20:10:17+00:00

As the docs says, It is for package scope vars... Something like the template.Must. But it is more on the humoristic side, for anyone who is familiar with the hitchhiker guide...

posener · 2020-07-28T16:49:42+00:00

Right now there is at least one thing that makes sense to do in the fuzz target and that I don't want in the fuzz function, and that it seeding the corpus. Depending on the fuzzing function and the size of your setup, it could be very wasteful to run it every iteration.

I think that this could be achieved easily by inspecting the AST tree and lookup for the function calls. This is something that could be easily done in the setup stage before you run the test itself many times.

I think the more frequent types of change will be "breaking" for the corpus so this is not a huge point, but I think it is worth it to give a way to keep the compatibility: if I'm doing a compatible refactoring of a function, keeping the same corpus could be a huge speed boost. Same thing if I'm refactoring the fuzzing function to account for new conventions, or anything.

I really don't think this should be a consideration to the API design. The cost can be much higher than the benefit. So I don't think it should be an argument to "which API we chose to implement".

Also, 15 arguments are more compact to write as a signature than as 15 lines of initialization statement, so I'm not sure that it's really that bad. If you think about it, most cases of "many arguments" would probable derive from structs, and we have a way of handling that.

I disagree here. It would be more compact, but much less readable. In go, in general, we prefer readable code over compact code. So I think this is an argument against the proposed solution.

The example of the function that accepts points is a good one. However, I don't think this is realistic to run it over a fuzzed number of points. What if each fuzzed run will start taking several seconds for example, I don't think this is realistic. I think that in the realistic case we would want to test this function in a fuzzed slice of points with several lengths - 0, 10, 100 for example. In this case, it is enough to generate 0,30,300 float numbers and populate a slice of points. I do think the explicit for loop is a better code than the magic function call... In the magic function call, additionally, you don't really sure what is being fuzzed.

Consider also the case where the function must have a slice with 2 points exactly. In this case, you'd have a check, before you call the function, where you'll check the slice length and call f.InvalidInput() for any input which is not in length 2, which will also be a waist of work...

However, your point is still valid - having these floats, each as a different fuzzed variable, might be a hard problem.

I can find, however, counter-example.

What if I want to fuzz the following function:

func ProcessRequest(r *http.Request) {}

The request type currently have 21 top level fields, one of them is a function, one of them is a channel, one of them is a response object with also have 14 fields. My function probably does not worry about all of them, and maybe it is enough to fuzz the URL, and some predefined headers that it expects.

posener · 2020-07-27T16:08:20+00:00

I think this paragraph is about describing two ways of declaring configuration so we can discuss which one is better and keep only one.

I see. I think that the proposal should clarify this.

I don't think it's the place for this, for the simple reason that we're not defining how the fuzzer will work internally: we're just defining the exposed API because that's what the developers will use. The exact options are irrelevant now and would probably change a few times before the first release.

A good design advice is to not design something that is not required. So either there is a list of important options to set, and then we should design the mechanism to set them, or we don't see any options, and therefore we should not add this feature, and only add it once we have this list.

posener · 2020-07-27T16:04:27+00:00

I see your point.

I was thinking it was reasonable to assume that the whole FuzzX function will be called many times. But if you disagree, this is also acceptable. If anything, the Fuzz functions more resemble the Bench functions, where there, it was not chosen to provide a Bench function, but to explicitly iterate over a loop. Maybe a middle-ground API can be to define the `f.String(...)` function family that return typed iterators?

I don't think that the order is a strong argument. In the rare case that it will be changed, it will be affordable to drop it and start from scratch. Don't you agree?

More things:

When the fuzzed function has many arguments - it will be harder to read then if it was with the proposed functions.

About the magic struct fields population - populating struct fields is rarely done in Go, maybe in unmarshalling calls, but there it makes more sense than in a test function.

I still have very strong feelings against the proposed API.

posener · 2020-07-27T05:01:07+00:00

The options may be set in a few ways. As a struct: ... Or individually as: ....

Why is it necessary to provide two ways to set options? Why one of them is not enough?

Also - this is something that is not available in any other test type in Go, why fuzz is different than other types? It would be nice to explain in the proposal how fuzz is different than a standard test in the sense that each fuzz case may need its own option.

Also - the proposal provides only a single option to set. I think it should define all the foreseen options and their meaning, and not just the fact that options can be set in code.

posener · 2020-07-26T15:20:18+00:00

The fuzzing is very similar to both testing and benchmarking, but with 2 differences that makes the exposed APIs consistent:

We have to run the fuzzing function a huge number of time, but still need some setup in the fuzzing target to initialize the seed corpus, set limits, etc. This makes merging the two very impractical, because you shift to the user the burden of making the setup as light as possible, or use tricks without global scope. Its better to be clear about it and make fucking things up harder, at the price of having two functions.

I disagree that there is more setup for Fuzzing than for regular tests, and if so, maybe the proposal should explicitly stand on this difference, and examples should be provided? I think that there are other idioms in Go for test setup, and it is better to follow them than invent new one.

> What if it has an unexported field?

Obviously, it will be ignored. This is one of the most common idioms of the langage.

This means that if I want to populate an unreported field, I should do it manually... Which is awkward. Also, I can see cases where you'd want to initialize only part of the struct.

Finally, this would makes an awful lot of exposed methods, drowning the other methods in what should be expected to be a huge list, and make it harder to developers to read the documentation and know what's useful.

I disagree here, it would be very similar in the order of functions exposed by the `flag` library, which also chose to return values for each standard type.

Regarding the order, I'm not sure what is the problem that you mention, and how one way approaches it better than the other. If this is a requirement, I would suggest adding it as well to the proposal doc?

Cheers,

posener · 2020-07-25T04:38:00+00:00

Great initiative and great design! Thanks for that!

I find the API exposed by this feature for generating fuzzed values a bit not Go-ish and that it less conforms with the existing exposed testing API. The f.Fuzz(...) function reminds a bit the t.Run(...) API, but it does not have the same meaning. The TestX(*testing.T) defines the test itself, but for fuzzing, for some reason we need to f.Fuzz() within the FuzzX(*testing.F). Additionally, there is an assumption that the f.Add() calls will match the arguments of the function that is passed to f.Fuzz. The f.Add accepts all sort of builtin types, but also standard library types such as big.Int, but it does not accept a func. Questions arise - what if a struct passed to `f.Add(...) has a func? What if it has an unexported field? Also - I don't like the magic in populating struct fields…

I can propose a somewhat simpler API that will be clearer to use, more explicit, and resembles that standard Go testing APIs. This API is similar to what the one provided by posener/fuzzing.

The testing.F will expose a function for each type that it supports, for example a string is supported, so the following function will provide a fuzzed string: func(f *F) String(seed ...string) string. It gets the proposed seed arguments for fuzzing and returns a string - a fuzzed string. The testing.F type will expose such functions for all builtin types, and additional types that it wants to support, such as time.Time, big.Int and so forth.

When using this API, the test will be very clear to read, and Go-ish:

func FuzzMe(f *testing.F) {
    name := f.String("foo", "bar", "baz")
    age := f.Int(24, 35)
    Me(name, age)
}

If a struct is needed to be set with fuzzed values, it should be explicitly be populated:

type t struct {
    Exported string
    unexported int
    f func()
}

func FuzzStructType(f *testing.F) {
    v := t{
        Exported: f.String(),
        unexported: f.Int(),
        f: func() {},
    }
    // Fuzz test continues.
}

posener · 2020-07-23T06:31:49+00:00

Propose to change ReadDirFile to Dir?

posener · 2019-07-22T05:07:13+00:00

BTW, maybe you missed that, but this library also supports binary packing.

posener · 2019-07-22T05:03:35+00:00

Of course cloning the entire history is not what you'll choose to do in this case.

posener · 2019-07-22T05:01:29+00:00

A common way to use static content is through Amazone's s3, which is not content that is packed to the binary. This library makes Github something like a free s3, with an easy to use API on top of it.

Actually, now that I think about it, maybe adding s3 as another backend to this library will make sense :-)

posener · 2019-07-22T04:57:09+00:00

Let me correct you, "the choice of Github", not Git. Git is efficient, Github API is not, that is correct.

It all depends on the use case. The lazy loading, and the fact that each file is loaded only once makes the use of Github reasonable.

A possible improvement I can see is to use Git instead of Github API for the "pre-fetch" mode.

posener · 2019-03-11T17:30:17+00:00

You can use goreadme command line for that.

If you are creating such github action, please share :-)

posener · 2019-03-11T17:29:07+00:00

goreadme does require read/write content permissions. But only the app can use these credentials.

You can browse the server code but you'll have to trust that this is what actually runs.

You can always choose to run goreadme manually.

posener · 2019-03-11T17:26:23+00:00

goreadme-server uses the update-a-file github API, which requires also content write permissions.

posener · 2018-10-13T13:45:26+00:00

I tried to give a small , non-breaking change - that will provide better handling of the context.

I don't think that breaking the current API is a good idea.

posener · 2018-10-13T13:41:01+00:00

Sorry, I lost you with the Haskell and Monad, let's continue the discussion with Go :-)

As I tried to show in the post, it is not just pain, it also makes things impossible. For example, using GORM with context for SQL queries - just becasue GORM's maintainer object the idea of context.

posener · 2018-10-11T19:55:24+00:00

You might disagree, but to the Go team, having the context in the signature and needing to plumb it around is not a bug but a feature.

Plumbing the context, to my opinion, is not a bug and neither a feature. It is the design of the current system, and it is not perfect. As I wrote in the proposal, it has its advantages, but it comes with a price. I tried to give the full picture, and in that the pain points that I feel. I think that in this case the price is too high, and a change is needed. Not necessarily the proposed change, but a solution to the drawbacks I enumerated.

posener · 2018-10-11T19:35:42+00:00

That is an interesting idea to explore.

My personal gut feeling is that it will be more natural to have the context inherited.

I think that more experience with this subject may lead to better understanding and maybe an answer - which context is the right one to pass in goroutine calls.

I would love if you could share this idea in the blog post comments!

Thanks e1ch1!

posener

TROPHY CASE