Hey @gavv can assign this to me? I am little busy with switching jobs but this problem seems interesting. Would pickup as soon as I get some time.

Sure, thanks!

Hey @gavv converting

type Number struct {
	noCopy noCopy
	chain  *chain
	value  float64
}

to

type Number struct {
	noCopy noCopy
	chain  *chain
	value  big.Float
}

will cause Raw() to fail. As it returns float64. The code won't compile unless we comment out the code instead of deprecating it or we can modify it to return float64 by converting it from big.Float. But if we decide to make changes on the function then we don't need to deprecate it. And AsFloat and Raw would be the same exact function. How do you suggest we proceed?

or we can modify it to return float64 by converting it from big.Float

Yes, that's what I was suggesting.

And AsFloat and Raw would be the same exact function

Yes.

Yeah, sorry, I was not clear on this. The idea is the following:

• we add AsFloat, it returns float64; we also add other AsXxx methods
• we keep Raw signature untouched, it still returns float64
• code that uses Raw keeps compiling and working, we don't break compatibility
• however we mark Raw as deprecated and users will get a warning that they should migrate from Raw to AsFloat (if they want float64), or to other AsXxx
• we wait for some time, e.g. a few months; during this period, users will hopefully update lib, see deprecation warning, and migrate to AsXxx methods; since it's a warning, not a error, they can do it gradually in comfortable pace
• after a few months, or maybe a year, I plan to release httpexpect v3, which will remove all currently deprecated functions
• in v3, we will change Raw to return big.Float instead of float64; this will break compatibility, but it's allowed in a major version upgrade; furthermore, since Raw was deprecated way before, many users wont be using Raw at this point, and their migration to v3 will be more smooth

This is actually a common approach for breaking APIs: deprecate, wait, then remove or change. Wait period gives users time to reacts to the change without hurry.

Hey @gavv I have few questions.

1. There are certain deprecated functions that uses newNumber() should those be replaced too?
2. rework JSON decoding:

find places where we decode JSON: canonValue, Response, WebsocketMessage; configure JSON decoder to use json.Number instead of float64 (see https://pkg.go.dev/encoding/json#Decoder.UseNumber)

ensure that canonValue() and other canonization functions does not cause precision loss

For ☝️ do you suggest to have a type assertion wherever the functions are called? Because as far as I can see. All of the canon functions are using interfaces to decode json rather than any specific type.

1. Yep

2. Sorry, I don't understand the question/suggestion, could you elaborate?

So let's suppose we have this function -

func canonValue(opChain *chain, in interface{}) (interface{}, bool) {
	b, err := json.Marshal(in)
	if err != nil {
		opChain.fail(AssertionFailure{
			Type:   AssertValid,
			Actual: &AssertionValue{in},
			Errors: []error{
				errors.New("expected: marshalable value"),
				err,
			},
		})
		return nil, false
	}

	var out interface{}
	if err := json.Unmarshal(b, &out); err != nil {
		opChain.fail(AssertionFailure{
			Type:   AssertValid,
			Actual: &AssertionValue{in},
			Errors: []error{
				errors.New("expected: unmarshalable value"),
				err,
			},
		})
		return nil, false
	}

	return out, true
}

How do you want it to be modified to use json.Number? Because from what I can see we are using interface{} as input.

Or Am I understanding this wrong? If so can you please explain this?

Ah, I see.

canonValue and other methods use json.Unmarshal to unmarshal object, array, etc. When json.Unmarshal encounters a number somewhere in json, it unmarshals it into float64. This may cause precision loss if the number can't be represented as float without loss (if it's a large 64-bit integer for example).

encoding/json provides an option to unmarshal numbers into json.Number instead of float64. (This need using json.Decoder instead of json.Unmarshal).

We need to use this option everywhere when we unmarshal json, to avoid precision loss. Our maps, arrays, etc. should have json.Number instead of float64.

When the user will create Number for such element, it will construct big.Float from json.Number.

Understood. Thanks. 😄

Some thoughts:

• To avoid code duplication, we need a function (I think in canon.go) that performs decoding with right options. We'll need to use it everywhere where we used json.Unmarshal before.

• If you wish, you can think about splitting your PR into several, so that we can merge some of them earlier, to avoid accumulating of conflicts and unnecessary work on resolving them. For example:

  • You could create a PR that just switches Number to big.Float internally, but does not change Number interface: constructor will still accept float, Raw will still returns float, methods like InDelta still accept float, etc etc. We can quickly review and merge this, and you can continue work on other parts of the code without gaining conflicts on this part.

  • Another example: you could create a PR that changes all calls to json.Unmarshal to calls to a function in canon.go, but without changing behavior (it can still use json.Unmarshal or Decoder without options). In future PRs you can change this function to use json.Number when the code base will be ready for it.

  • Another example: DefaultFormatter will need adjustments to support big floats and json numbers. This can also be done in a separate PR.

Sure thing. Then let me do one thing. I will make the current changes pass the checks, so that these can be merged. Later on I will divide newer development into multiple tasks.

Note that if you change decoding to use json.Number, it will break many things, and it will take time to fix all of them. I'd recommend to cut it from first PR.

But I already started making those fixes. 😅

Oh I see, great :)

Hey @gavv

Test cases are failing in number_test.go. For example

	t.Run("Decode into empty interface", func(t *testing.T) {
		reporter := newMockReporter(t)

		value := NewNumber(reporter, 10.1)

		var target interface{}
		value.Decode(&target)

		value.chain.assertNotFailed(t)
		assert.Equal(t, *big.NewFloat(10.1), target)
	})

Below is the output of test case.

--- FAIL: TestNumber_Decode (0.00s)
    --- FAIL: TestNumber_Decode/Decode_into_empty_interface (0.00s)
        number_test.go:77:
                Error Trace:    number_test.go:77
                Error:          Not equal:
                                expected: big.Float(big.Float{prec:0x35, mode:0x0, acc:0, form:0x1, neg:false, mant:big.nat{0xa199999999999800}, exp:4})
                                actual  : map[string]interface {}(map[string]interface {}{})
                Test:           TestNumber_Decode/Decode_into_empty_interface

The error is caused because big.Float is not parsed correctly to json in canonDecode method, since all the numbers are right now converted to big.Float. Any idea how to decode big.Float or big.Int to json?

I think you need to convert big.Float to json.Number and then pass it to json decoder.

I think you need to convert big.Float to json.Number and then pass it to json decoder.

I don't think we can directly convert big.Float to json.Number because big.Float is a struct. Rather I was using a reverse conversion and then passing it to json decoder. Check out this snippet -

func canonNumberDecode(opChain *chain, value big.Float, target interface{}) {
	if target == nil {
		opChain.fail(AssertionFailure{
			Type: AssertUsage,
			Errors: []error{
				errors.New("unexpected nil target argument"),
			},
		})
		return
	}
	t := reflect.Indirect(reflect.ValueOf(target)).Kind()
	fmt.Println("type", t)
	switch t {
	case reflect.Float64:
		f, _ := value.Float64()
		canonDecode(opChain, f, target)
	case reflect.Float32:
		f, _ := value.Float32()
		canonDecode(opChain, f, target)
	case reflect.Int8:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Int16:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Int32:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Int64:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Int:
		i, _ := value.Int64()
		fmt.Println("int", i)
		canonDecode(opChain, i, target)
	case reflect.Uint8:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Uint16:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Uint32:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Uint64:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Uint:
		i, _ := value.Int64()
		canonDecode(opChain, i, target)
	case reflect.Interface:
		f, _ := value.Float64()
		canonDecode(opChain, f, target)
	default:
		canonDecode(opChain, value, target)
	}
}

This would work too... does it handle wrapped types, e.g. type myInt int?

I'd rather use json.Number + json.Unmarshal because it's less code and we can be sure that we didn't miss any details.

json.Number is just a wrapped string, so "conversion" is something like json.Number(value.String()). Though, we should check that it never loses precision and how it handles big numbers.

This would work too... does it handle wrapped types, e.g. type myInt int?

I'd rather use json.Number + json.Unmarshal because it's less code and we can be sure that we didn't miss any details.

json.Number is just a wrapped string, so "conversion" is something like json.Number(value.String()). Though, we should check that it never loses precision and how it handles big numbers.

t := reflect.Indirect(reflect.ValueOf(target)).Kind() this takes care of wrapped types.

The problem with using json.Number and json.Unmarshal is json.Number uses string. But the target can be of any type. So if an user wants to convert big.Float to int. Decode function will fail - error: json: cannot unmarshal string into Go value of type int. Which is why the canonNumberDecode function is required.

Hey @gavv there's one more issue. There is no NaN representation in big.Float/big.Int . There is only a type ErrNaN (https://pkg.go.dev/math/big#ErrNaN) that panics if the input is NaN. How do you suggest we go about this issue? For example -

	v1 := NewNumber(reporter, math.NaN())

The above snippet in TestNumber_EqualNaN fails. Although I was thinking to use big.NewFloat(0) as a value but then

	v6 := NewNumber(reporter, math.NaN())
	v6.NotInDelta(1234.0, 0.1)
	v6.chain.assertFailed(t)

this portion of the test case fails. Can I use -Inf as NaN representation in big.Float? If yes, then I have to write couple of extra checks on each method to see if the number is -Inf which in this case would be NaN value representation in big.Float.

t := reflect.Indirect(reflect.ValueOf(target)).Kind() this takes care of wrapped types.

Oh, cool.

The problem with using json.Number and json.Unmarshal is json.Number uses string. But the target can be of any type. So if an user wants to convert big.Float to int. Decode function will fail - error: json: cannot unmarshal string into Go value of type int. Which is why the canonNumberDecode function is required.

Sorry, I don't know why I mentioned json.Number at all, it has nothing to do with this.

What I was trying to say is that we can convert big.Float to string and use stock json decoder, like this:

https://go.dev/play/p/uyl7R19QBYa

Can this work for us?

Regarding NaN.

We can't use any other floating point value to represent NaN: zero, -inf, +inf - all are valid values that can be passed by user and we should not confuse them with NaN.

The only option we have is to handle NaN specially.

Since it seems that NaN is the only unsupported value (both -Inf and +Inf are seem to be supported), I think the easiest way to handle it is to use *big.Float instead of big.Float, and use nil to represent NaN.

Then we'll need to check for nil in all methods. And I guess we'll need to add NaN tests for all methods.

Hey @gavv#6022 I don't think our approach is right. For example :-
Look at the test function -

func TestArray_Decode(t *testing.T) {
	t.Run("target is empty interface", func(t *testing.T) {
		reporter := newMockReporter(t)

		testValue := []interface{}{"Foo", 123.0}
		arr := NewArray(reporter, testValue)

		var target interface{}
		arr.Decode(&target)

		arr.chain.assertNotFailed(t)
		assert.Equal(t, testValue, target)
	})

	t.Run("target is slice of empty interfaces", func(t *testing.T) {
		reporter := newMockReporter(t)

		testValue := []interface{}{"Foo", 123.0}
		arr := NewArray(reporter, testValue)

		var target []interface{}
		arr.Decode(&target)

		arr.chain.assertNotFailed(t)
		assert.Equal(t, testValue, target)
	})

	t.Run("target is slice of structs", func(t *testing.T) {
		reporter := newMockReporter(t)

		type S struct {
			Foo int `json:"foo"`
		}

		actualStruct := []S{{123}, {456}}
		testValue := []interface{}{
			map[string]interface{}{
				"foo": 123,
			},
			map[string]interface{}{
				"foo": 456,
			},
		}
		arr := NewArray(reporter, testValue)

		var target []S
		arr.Decode(&target)

		arr.chain.assertNotFailed(t)
		assert.Equal(t, actualStruct, target)
	})

	t.Run("target is unmarshable", func(t *testing.T) {
		reporter := newMockReporter(t)

		testValue := []interface{}{"Foo", 123.0}
		arr := NewArray(reporter, testValue)

		arr.Decode(123)

		arr.chain.assertFailed(t)
	})

	t.Run("target is nil", func(t *testing.T) {
		reporter := newMockReporter(t)

		testValue := []interface{}{"Foo", 123.0}
		arr := NewArray(reporter, testValue)

		arr.Decode(nil)

		arr.chain.assertFailed(t)
	})
}

Check the var target - it can either be interface{} or []intreface{}. Moreover the array can have structure as []interface{}{} or []interface{}{map[string]{}, {}interface{}, []interface{}} .

Now when we decode the function will internally call

func jsonDecode(opChain *chain, b []byte, target interface{}) {
	reader := bytes.NewReader(b)
	dec := json.NewDecoder(reader)
	dec.UseNumber()

	for {
		if err := dec.Decode(target); err == io.EOF || target == nil {
			break
		} else if err != nil {
			opChain.fail(AssertionFailure{
				Type:   AssertValid,
				Actual: &AssertionValue{target},
				Errors: []error{
					errors.New("expected: value can be decoded into target argument"),
				},
			})
			return
		}
	}
}

To handle the issue I wrote something like

func (a *Array) Decode(target interface{}) *Array {
	opChain := a.chain.enter("Decode()")
	defer opChain.leave()

	if opChain.failed() {
		return a
	}

	canonDecode(opChain, a.value, target)

	if targetPointer, ok := target.(*interface{}); ok {
		if data, ok := (*targetPointer).([]interface{}); ok {
			for i, d := range data {
				if v, ok := d.(json.Number); ok {
					if f, err := v.Float64(); err == nil {
						fmt.Println("json", f)
						if hasPrecisionLoss(f) {
							numStr := v.String()
							num, _ := big.NewFloat(0).SetString(numStr)
							data[i] = num
						} else {
							fmt.Println("json", f)
							data[i] = f
						}
					}
				}
			}
			*targetPointer = data
		}
	}

	return a
}

But ☝️ will only work when the target is {}interface and a.value is []interface{} Otherwise it will fail.

Now, I don't see any way to convert json.Number back to either float64 or big.Float without manually checking for all the cases where the structure may vary.
How do you suggest we keep check for all these scenarios with too many variable structure?
Moreover this will introduce inconsistency. What do you think?

Hi, and sorry for delay.

When we were discussing this in chat, my thoughts were the following (I think I didn't explain all of the idea accurately enough):

• when we decode json from http response (in Response.JSON), we ask decoder to use json.Number; we'll get a map or slice or whatever object that occasionally may have json.Numbers inside

• after decoding, we perform canonization (in Value constructor we call canonValue; we also call canon funcs in all other constructors);

• during canonization, as suggested, we should replace json.Number with either float64 (if it's small) or big.Float (if it's large), hence preserving compatibility in cases when we were working correctly before; to achieve this, we should recursively inspect the object (map, slice, map of maps, whatever), find all json.Number instances, and convert them (I guess using reflect)

• as discussed, the behavior of the previous step is controlled by an option; user can choose: always use float64 (will be default in v2), always use big.Float, automatic selection based on needed precision (will be default in v3)

• after canonization, Value (and other structs like Object, Array, etc; everything except Number) will internally represent numbers as float64 or big.Float

• when user calls Raw (e.g. Value.Raw, Object.Raw), they will get objects (maps, slices, etc) with numbers as float64 or big.Float; similarly, when user calls operations like IsEqual(), the operations will work with same representation of numbers

• when user calls Decode (e.g. Value.Decode), it internally calls canonDecode; here we need special handling: we should again recursively find all numbers, convert them to json.Number, and the decode value into user's target using json decoder;

• when user will eventually construct Number (e.g. call Value.Number() or Value.Array().Value(0).Number(), etc), Number constructor will receive interface{} for the numeric value

• Number constructor will inspect received interface{}; it will be either float64 or big.Float; however Number will unconditionally convert it to big.Float, if it's not NaN, or to nil if it's NaN

• all Number operations will work with big.Float that is stored internally; from user arguments, these operations (e.g. IsEqual) will accept interface{} that can be integer, float, or big.Float

• Number.Raw will still return float64 for compatibility (and will be deprecated), however in v3 we'll change it to return big.Float (and will undeprecate it)

A lot of bullet points... but I tried to give comprehensive overview this time. In short, there are actually only a few key points:

• during decoding from HTTP, use json.Number (as you already do)

• during canonization, everywhere except Number, recursively find json.Number and covert to float64 or big.Float (new feature)

• during decoding (in canonDecode), recursively find float64 and big.Float and covert to json.Number, then encode to json and decode into target (new feature)

• during canonization in Number constructor, accept both float64 and big.Float, and convert to big.Float (as you already do)

And that's it.

Does this solve the problem you mentioned?

BTW I want to thank you for patience and all these long discussions on discord. I was not expecting all the obstacles when creating the task, and I think these discussions have generated a rather good solution.

Hi, and sorry for delay.

When we were discussing this in chat, my thoughts were the following (I think I didn't explain all of the idea accurately enough):

• when we decode json from http response (in Response.JSON), we ask decoder to use json.Number; we'll get a map or slice or whatever object that occasionally may have json.Numbers inside
• after decoding, we perform canonization (in Value constructor we call canonValue; we also call canon funcs in all other constructors);
• during canonization, as suggested, we should replace json.Number with either float64 (if it's small) or big.Float (if it's large), hence preserving compatibility in cases when we were working correctly before; to achieve this, we should recursively inspect the object (map, slice, map of maps, whatever), find all json.Number instances, and convert them (I guess using reflect)
• as discussed, the behavior of the previous step is controlled by an option; user can choose: always use float64 (will be default in v2), always use big.Float, automatic selection based on needed precision (will be default in v3)
• after canonization, Value (and other structs like Object, Array, etc; everything except Number) will internally represent numbers as float64 or big.Float
• when user calls Raw (e.g. Value.Raw, Object.Raw), they will get objects (maps, slices, etc) with numbers as float64 or big.Float; similarly, when user calls operations like IsEqual(), the operations will work with same representation of numbers
• when user calls Decode (e.g. Value.Decode), it internally calls canonDecode; here we need special handling: we should again recursively find all numbers, convert them to json.Number, and the decode value into user's target using json decoder;
• when user will eventually construct Number (e.g. call Value.Number() or Value.Array().Value(0).Number(), etc), Number constructor will receive interface{} for the numeric value
• Number constructor will inspect received interface{}; it will be either float64 or big.Float; however Number will unconditionally convert it to big.Float, if it's not NaN, or to nil if it's NaN
• all Number operations will work with big.Float that is stored internally; from user arguments, these operations (e.g. IsEqual) will accept interface{} that can be integer, float, or big.Float
• Number.Raw will still return float64 for compatibility (and will be deprecated), however in v3 we'll change it to return big.Float (and will undeprecate it)

A lot of bullet points... but I tried to give comprehensive overview this time. In short, there are actually only a few key points:

• during decoding from HTTP, use json.Number (as you already do)
• during canonization, everywhere except Number, recursively find json.Number and covert to float64 or big.Float (new feature)
• during decoding (in canonDecode), recursively find float64 and big.Float and covert to json.Number, then encode to json and decode into target (new feature)
• during canonization in Number constructor, accept both float64 and big.Float, and convert to big.Float (as you already do)

And that's it.

Does this solve the problem you mentioned?

BTW I want to thank you for patience and all these long discussions on discord. I was not expecting all the obstacles when creating the task, and I think these discussions have generated a rather good solution.

Now I understand. I although have an issue with the overhead of conversion. But I think this is inevitable if we are keep it compatible. In v3 we can remove this overhead easily as we will be allowed to have breaking changes.
But this makes sense. This was a clear explanation, thanks.

Even I wasn't expecting so many hurdles. But I guess it's good that had this detail conversation. It is much clear now.

Regarding overhead, I suppose it's comparably with json marshalling and unmarshalling, which is as well based on reflect. So here we don't change much in terms of performance.

Regarding overhead, I suppose it's comparably with json marshalling and unmarshalling, which is as well based on reflect. So here we don't change much in terms of performance.

Got it. Btw for now what I am following is every number that will not cause precision loss will be converted to float64 by default and numbers which will cause precision loss will be converted to big.Float. And this will be the default behaviour for now. Once I fix all the test cases and this PR is merged. I will add a config var which will use either float64 or big.Float as per the user choice. I think this would be the ideal way to go forward. Otherwise it will increase complexity to a already complex PR.
Also the PR is getting huge in terms of code changes. 😅