search

algorand / pyteal

Algorand Smart Contracts in Python
https://pyteal.readthedocs.io
MIT License
285 stars 131 forks source link

working through a unit test #682

Closed tzaffi closed 1 year ago

tzaffi commented 1 year ago

DO NOT MERGE!

❯ diff tests/teal/router_bug/*
42,44d41
< intc_0 // 0
< dup
< bytec_0 // ""
tzaffi commented 1 year ago

@bbroder-algo @ahangsu

Upon reflecting some more on this PR, I decided to close it and not convert it into a new test in the repo. Let me explain my findings and reasonings.

  1. The test here injects a superfluous piece of code in the case that the superfluous member CompileOptions.TEMP4DEBUG is True
  2. We can see that this injection causes an actual difference in the compiled Teal
  3. I verified locally, that when I merged this branch router-test into current master (commit hash d1961a6ef3ba1138aa9a9b400afa7e65edd6b744 ) all tests actually passed, so that no difference resulted from this artificial condition
  4. It is unreasonable to maintain such a change to production code, just to show that the artificial situation produces no bug
  5. It is probably possible through python AST magic to create a test that reproduces the same without actually changing any committed source code; however, the test is likely to be non-trivial to write, very fragile and a maintenance nightmare.
  6. Since #684 has addressed the original concern of the test, and also includes a kind of variant of this test we should follow the Pareto Principle and declare victory