pytest-dev / regendoc

Mozilla Public License 2.0
2 stars 8 forks source link

Replacing addresses with `0xdeadbeef` is a bit problematic in some situations #12

Closed nicoddemus closed 3 years ago

nicoddemus commented 3 years ago

As https://github.com/pytest-dev/pytest/pull/8274 shows, in that particular example we show two instances which are supposed to be different, however they both end up with the same address which is confusing.

Perhaps regendoc could be more sophisticated here, keeping track of memory addresses used in the code but generating a fixed memory address based on the contents somehow?

RonnyPfannschmidt commented 3 years ago

I believe the replacement is part of the pytest config

Smart address replacement needs a bit of help

nicoddemus commented 3 years ago

I believe the replacement is part of the pytest config

Hmmm do you remember where? I don't see it here: https://github.com/pytest-dev/pytest/blob/fff9f28fdd9d7dd21ae6bed94ee7ee565b7b5839/doc/en/Makefile#L23-L33

(EDIT) Seems to be hard-coded in regendoc:

https://github.com/pytest-dev/regendoc/blob/27e3652723659d8b2d7a5311567480fccc49cbe2/regendoc/__init__.py#L74-L80

Smart address replacement needs a bit of help

What do you mean by "a bit of help"? Someone contributing the code to do that, or something else?

RonnyPfannschmidt commented 3 years ago

a code address substituter is needed that picks different replaces

0bsearch commented 3 years ago

I'd would like to help with this. Maybe as a first question: can we challenge the fact memory address Substituter exists? Was random addresses actually a problem?

If we still want to keep it, do you have any vision on how to tackle this? I guess, one approach would be to have replace receive iterable to take data from.

Analyzing page/codeblock for different addresses to understand that we should keep that untouched is not an option, as it will defy the purpose – as it will leave real random addresses as is.

RonnyPfannschmidt commented 3 years ago

Each regen run has a new set of random addresses

The address substitution ought to be replaced with something that supports more than one unique value as output

RonnyPfannschmidt commented 3 years ago

fixed as part of #13