Do we need objx dependency?

[ccoVeille]

trafficstars

This question follows this PR

• https://github.com/stretchr/testify/pull/1567

• https://github.com/stretchr/objx/pull/140

Context: objx depends on testify for its tests, testify depends on objx for the code (I didn't check where).

I opened to solve the issue on objx side.

• https://github.com/stretchr/objx/pull/154

But maybe another approach is to remove this dependency?

cc @dolmen @brackendawson

[brackendawson]

I don't think we ever needed it. It's used to embed a scratchpad in structs which embed mock.Mock. But anyone that needs a scratchpad could trivially add one themselves.

We can't remove it because it's a breaking change.

[dolmen]

We could start with a deprecation of Mock.TestData.

What do you think?

[ccoVeille]

Go for it, it doesn't sounds much used in public repositories

https://github.com/search?q=language%3Ago+testify+%22mock.testdata%22+-is%3Afork+-is%3Aarchived+-org%3Astretchr+-path%3A%2Ftestify%2F&type=code

[dolmen]

This code search returns more relevant results (but confirms the rare use):

https://github.com/search?q=language%3AGo+%22ock.TestData%28%29%22+-is%3Afork&type=code

[dolmen]

Here is a use of TestData (in project grafana/tanka) that can just be replaced by storing test data directly in the mock object. https://github.com/grafana/tanka/pull/1467/files

[brackendawson]

We would still need to bump the major version number to remove this else risk breaking innocent projects' builds.

Deprecating it isn't so bad as there is an easy alternative of simply embedding the objx.Map in the struct next to mock.Mock. But we will still have the dependency, so I don't think there is actually any plus side to justify the (relatively minor) pain.

[dolmen]

While we can't just remove the objx dependency in v1 because of TestData() API breakage, we can still make it easier for downstream users to not dependend on it. I already did related things for YAML in #1579.

Here is a plan:

1. Move objx-dependent code in their own source files. That will ease the maintenance of downstream forks of Testify that would remove objx
2. Add compile tags allowing to compile testify/mock without TestData
3. Deprecate TestData with clear documentation about the reason of the deprecation and with workaround examples.

[dolmen]

Step 1 is implemented in #1757.

[emilien-puget]

solved the issue on objx side https://github.com/stretchr/objx/pull/159

[brackendawson]

solved the issue on objx side stretchr/objx#159

This only solves the cyclic dependency. Testify v1 must import objx forever.

[emilien-puget]

solved the issue on objx side stretchr/objx#159

This only solves the cyclic dependency. Testify v1 must import objx forever.

The root cause of this whole discussion is the cyclic dependency

[ccoVeille]

solved the issue on objx side stretchr/objx#159

This only solves the cyclic dependency. Testify v1 must import objx forever.

Yes

This only solves the cyclic dependency. Testify v1 must import objx forever.

The root cause of this whole discussion is the cyclic dependency

Yes.

That said, I feel like everyone is right 😁

It's just a way to share the status.

So I would sum it up like this:

How was previous situation?

A real problem.

Would it be better with the fix ?

Yes

Would it be enough for fixing the cyclic dependencies between testify and objx ?

Yes

Should we consider the issue with the fact objx was imported solved ?

No, as it brings a dependency for almost nothing.

Is it better ?

Yes.

Is it OK?

Not without a v2, v1 will use objx as a dependency forever.

So I would say thanks @emilien-puget !

It's a solution that would simplify things, and it addressed what was reported in this issue.