Avoid mutation of TTFont to fix issues with concurrent tests

[Hoolean]

Although the mutation is reversed, it could still have interfered with TTFont instances being used concurrently by other tests.

Before undrafting, I will test to see if this fixes #4834.

(even if not, the change may prevent similar issues in the future, and so we may wish to merge anyway)

Checklist

• Update CHANGELOG.md
• Wait for the tests to pass
• Request a review

[Hoolean]

Failing pytest appears to be unrelated; has a dependency bumped somewhere?

[Hoolean]

Undrafting as the PR appears to fix the race condition for me locally (although I may have just gotten lucky)

[felipesanches]

I'll admit that I'm a bit unsure about this. By assigning a deepcopy of ttFont to itself aren' t we still overwriting the original object? I suppose we may need to use a different name. Something like my_ttFont = deepcopy(ttFont). Does it make sense? I'm not sure if that's exactly how this works.

[Hoolean]

My understanding is that Python gives function parameters their own independent variable to store their reference to an object, but agree that it's 100% worth double-checking; I had a look just now to be safe, and it looks like we're all good 🎉

from dataclasses import dataclass

@dataclass
class Wrapper:
    value: int

def replacer(parameter: Wrapper) -> None:
    parameter = Wrapper(value=2)

outside = Wrapper(value=1)
print(outside)  # prints Wrapper(value=1)
replacer(outside)
print(outside)  # prints Wrapper(value=1) ✅

I considered giving the variable a new name, but thought that reusing it would avoid extra refactoring and would have the bonus of making it impossible for the rest of the function to use the pre-copied object by mistake.

[felipesanches]

Sorry, I've just pushed a few commits changing the names, but I had not seen these messages before doing so. I think I'll revert my additional commit, then.

Failing pytest appears to be unrelated;

Just to be sure we're on the same page, what was the pytest failure message in this case?

[Hoolean]

No worries, thanks for taking a look Felipe! 🚀

Yes, reverting sounds good - the test that I am seeing ERROR locally is unreachable_glyphs, which appears to be the one upsetting the CI too, with full error:

AttributeError: 'TTFont' object has no attribute 'glyphOrder'. Did you mean: 'getGlyphOrder'?

I thought it was unrelated before, but now I see that it is one of the checks that we have touched in this PR.

I think what might be happening is that the check previously populated .glyphOrder as a side-effect, but now that we deepcopy it is not happening any more; this is probably a good thing, so I think the pytest just needs to be updated to call .getGlyphOrder() instead of trying to access the raw property to accommodate this :)

[Hoolean]

It turned out to be a little more complicated because the test relied on certain context to remove a glyph; I think I have a full fix with fe7ab5e though (fuller explanation there too)