-
Notifications
You must be signed in to change notification settings - Fork 2
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
Test/fixing unit tests #248
Conversation
…use it was failing weirdly and appears duplicative of other test case
babylon.yaml
Outdated
@@ -0,0 +1,13 @@ | |||
babylon_binary: /usr/local/bin/bbi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was committed by accident and removed in the next commit
…ted out code (from 2 years ago)
…s because setMissingValuesToDefault no longer touches shrinkage
… integration test (and its totally broken) and the same functionality is tested in integration/bbi_summary_test.go
@elisarver feel free to take a look, and to start on #244 as soon as this is merged. But I do think we should have @dpastoor give this a lookover to make sure I didn't remove anything that he thinks should stay. |
@dpastoor I tried to keep the commits pretty clean, in terms of which tests/files they address, so that might be the easier way to go through this, as opposed to looking at the raw file diff view. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to help with the clean-up ahead of this, good changes.
t.R.Equal(3, len(test.modelOutput.ParametersData[0].RandomEffectSDSE.Omega), "Fail :"+test.context) | ||
setMissingValuesToDefault(&test.modelOutput) | ||
t.R.Equal(7, len(test.modelOutput.ParametersData[0].StdErr.Theta), "Fail :"+test.name) | ||
t.R.Equal(2, len(test.modelOutput.ParametersData[0].RandomEffectSD.Sigma), "Fail :"+test.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test.name is covered in all of these cases by the tt.Run above. it is reported as Test<Name>/7_thetas_3_omegas_2_sigmas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, so can we just get rid of that , "Fail :"+test.name
part of each line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup!
…lues_to_default_test.go per @elisarver review
Closes #239