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

Improve Testing for GlassBR #347

Closed
niazim3 opened this issue Jul 7, 2017 · 15 comments
Closed

Improve Testing for GlassBR #347

niazim3 opened this issue Jul 7, 2017 · 15 comments
Assignees

Comments

@niazim3
Copy link
Collaborator

niazim3 commented Jul 7, 2017

Mentioned in issue #262, there is a lot of duplication that can be reduced from the Testing code for GlassBR. As stated by @palmerst, "each test suite has 5-10 source files which are just complete copies of each other with different hardcoded inputs".
This issue will be used to keep track of changes made in relation to this task.

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 12, 2017

image
Current output.

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 12, 2017

As of commit 682b22f, make test no longer crashes. Current output:
image.
Work on getting all 93 tests to pass.

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 17, 2017

@palmerst Currently, the parameterized program is as follows in my working directory:
image, which is called by the original file calcTests.py as follows:
image, where the input file is the following:
calculations.txt.

Attempting to make the files results in the following error:
image.
How can the the initialization of the TestCalculations class be altered to avoid this error?

@palmerst
Copy link
Collaborator

palmerst commented Jul 17, 2017

In calcTests.py line 38, you have double parenthesized the TestCalculations constructor arguments -- so you are passing a length 10 tuple instead of 10 arguments. Remove the inner parentheses.

@palmerst
Copy link
Collaborator

Commit your changes to the tests and I'll take a look.

niazim3 added a commit that referenced this issue Jul 17, 2017
@palmerst
Copy link
Collaborator

I ended up making some big changes to the testing in d4c013a. The unittest framework does not like it when you redefine the constructor for unit tests. That is what was causing problems. Take a look at testCalculations.py -- it's much simpler now.

I moved all of the old tests to the OldTests folder in Python_Simplified

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 18, 2017

Update: commits 78fefda...1e9d557 contribute to this issue, since data has been pulled out from most duplicate files (except for files from testReadTable and testInterp).

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 26, 2017

Update:
testReadTable, testOutputFormat, and testMainFun, are the only tests broken. Working on getting these fixed, then, this issue can be closed...
Update (28/07/2017): testInterp test needs to be added, testMainFun has 1 failure, and testReadTable is broken`.

@niazim3
Copy link
Collaborator Author

niazim3 commented Jul 28, 2017

@palmerst The commit mentioned above made testOutputFormat produce the desired output, however, the calculations based off of the aspect ratio value were reducing in precision, which I don't believe is something that we'd like to lose (which is why I reverted how it was calculated in derivedValues.py). Do you have any suggestions on how this problem can be approached?

@palmerst
Copy link
Collaborator

Remove the rounding in derivedValues.py and change the output txt files that you are comparing against to match the precision printed by the program.

@JacquesCarette
Copy link
Owner

Looks like a lot of really good work was done here, and is close to being 'done'. @niazim3 could you document (your memory of) the state of this work? In a way that someone else could take it over and finish it?

@niazim3
Copy link
Collaborator Author

niazim3 commented May 8, 2018

I'm having difficulty finding the test cases that I'd like to review so I can update this issue in a more maintainable way. Where has the CaseStudies/glass/Implementations/Python_Simplified folder been transferred to? @JacquesCarette

@smiths
Copy link
Collaborator

smiths commented May 8, 2018

@niazim3, the case studies have been moved to the Case Studies repo.

https://github.com/smiths/caseStudies/tree/master/CaseStudies/glass/Implementations/Python_Simplified

This is to have a proper separation of concerns between the case studies and the Drasil implementation. However, it does cause a problem for issues like this. My suggestion is that you create a new issue in the Case Studies repo about this topic, and then close this issue with a copy of the url for the new issue.

@niazim3
Copy link
Collaborator Author

niazim3 commented May 9, 2018

The remainder of this issue will be worked on in the CaseStudies repo.

(@smiths I was unsure who to assign the issue to, so I have myself currently assigned. I can continue to work on this issue if it's my task, otherwise the issue can be re-assigned)

@niazim3 niazim3 closed this as completed May 9, 2018
@smiths
Copy link
Collaborator

smiths commented May 10, 2018

@niazim3, thank you for moving the issue. Yes, you should assign it to yourself. 😄

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

4 participants