Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 1.0.1 Testing #606

Closed
hdoupe opened this issue Aug 7, 2017 · 5 comments
Closed

Version 1.0.1 Testing #606

hdoupe opened this issue Aug 7, 2017 · 5 comments

Comments

@hdoupe
Copy link
Collaborator

hdoupe commented Aug 7, 2017

I discovered several bugs in the taxbrain interface. I used the requests library as suggested by @talumbau in issue #602 to create reforms on the test webapp and the local instance of the webapp. I uploaded the script post_reforms.py on my webapp-public branch.

I altered every parameter in the taxbrain interface, did a HTTP POST on my local webapp instance, copied the keywords to dropq from the celery log, and compared the expected dictionary to the actual dictionary in hdoupe/webapp-public/reform_input_processing/gen_reforms.assert_reforms_equal() I think this could be better done using the functions in test_views.py and I will spend time going over that tomorrow.

Tomorrow, I will clean up the scripts that I posted above and set up tests for the bugs listed above. I will also look into varying the start year and uploading reform files. Hopefully this first round of testing will be finished by the end of the day tomorrow.

@martinholmer @PeterDSteinberg @brittainhard

@hdoupe hdoupe changed the title Version 1.0.1 Testing Version 1.0.0 Testing Aug 7, 2017
@hdoupe
Copy link
Collaborator Author

hdoupe commented Aug 7, 2017

On completion of the tasks I set out for tomorrow and further input here, I will send out the email as required in the RELEASE_PROCESS.md

@hdoupe hdoupe changed the title Version 1.0.0 Testing Version 1.0.1 Testing Aug 7, 2017
@martinholmer
Copy link
Contributor

@hdoupe said:

_AMT_CG_rt{1,2,3} -- if I alter _CG_rt{i} then the corresponding rate _AMT_CG_rt{i} is pinned to the former rate, i = 1, 2, and 3. But _AMT_CG_rt4 is not affected. If _CG_rt{1,2,3} are not altered then _AMT_CG_rt{1,2,3} are correctly set to the specified values. See http://ospc-taxes7.herokuapp.com/taxbrain/edit/1406/?start_year=2017 and http://ospc-taxes7.herokuapp.com/taxbrain/edit/1407/?start_year=2017

I may be wrong and this could be a feature and not a bug. @martinholmer what are your thoughts here?

I'm not sure how TaxBrain is suppose to work, but I have found the paper trail on this matter.

First, look at item 4 in Tax-Calculator issue 1074 and item 4 in Tax-Calculator pull request 1109, which was merged on December 22, 2016.

Then look at TaxBrain pull request #448 (merged on January 12, 2017), which was supposed to be the fix on the TaxBrain side. However, the #448 TaxBrain changes did not eliminate the AMT_fixup logic in views.py, which had been added earlier on September 26, 2016, in pull request #342.

So, maybe I'm reading too fast, but it seems as if the behavior you've found was once considered a "feature", then became to be viewed as a "bug", but was never removed from TaxBrain. You're doing a great job on this TaxBrain debugging, so I'll leave it to you to read these issues and pull requests more carefully than I have to see what the intention was. Only then will you be able to determine if this behavior is a "feature" or a "bug".

@talumbau
Copy link
Member

talumbau commented Aug 8, 2017

@hdoupe this is great stuff. Some suggestions/misc. items:

  • what you've built so far is great for "parameter sweep" type runs and/or checking various corner cases. My advice is to be cautious with how many tests you run at a time. Each POST is the same as a TaxBrain user hitting the submit button, so for large number of submits, I would suggest running such jobs at off-peak hours. In doing such things like this in the past, it's quite easy to submit 100 jobs in just a few seconds.

  • I would file a separate bug for each item you find so it can be tracked separately.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Aug 8, 2017

@martinholmer said

So, maybe I'm reading too fast, but it seems as if the behavior you've found was once considered a "feature", then became to be viewed as a "bug", but was never removed from TaxBrain

Thanks for your feedback. I agree with your interpretation that this used to be a feature but now it is probably a bug. I will open a separate issue addressing this bug and a PR with a test exposing this bug.

@talumbau said

My advice is to be cautious with how many tests you run at a time. Each POST is the same as a TaxBrain user hitting the submit button, so for large number of submits, I would suggest running such jobs at off-peak hours. In doing such things like this in the past, it's quite easy to submit 100 jobs in just a few seconds.

Thanks for the heads up. It would have been unfortunate if I'd swamped the test webapp or blown up my computer.

I would file a separate bug for each item you find so it can be tracked separately.

Good idea. I will do this.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Aug 18, 2017

I'm closing this since I sent out an email documenting these results in the list serve. Issue #615 and PR #627 address the scripts I mention here:

I uploaded the script post_reforms.py on my webapp-public branch.

I altered every parameter in the taxbrain interface, did a HTTP POST on my local webapp instance, copied the keywords to dropq from the celery log, and compared the expected dictionary to the actual dictionary in hdoupe/webapp-public/reform_input_processing/gen_reforms.assert_reforms_equal()

@hdoupe hdoupe closed this as completed Aug 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants