-
Notifications
You must be signed in to change notification settings - Fork 21
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
HitOrMissMonteCarlo Test Added with Minor change in timeout setting #35
Conversation
@tdhock @faizan-khan-iit I've added a test for Hit or Miss Monte Carlo Integration and Pi value estimation. |
x1_nodes <- getNodeSet(info$html, "//g[@class='geom2_point_vizone']/g[@class='PANEL1']/circle/@cx") | ||
y1_nodes <- getNodeSet(info$html, "//g[@class='geom2_point_vizone']/g[@class='PANEL1']/circle/@cy") | ||
|
||
Sys.sleep(1.739) |
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.
why this number ?
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.
I took the reference from other successful tests. Sys.sleep()
is used to pause the program for the specified time so that next reading can be taken. I used the same time as in other tests.
great start but would be even better if you could edit the .travis.yml config to fix the build system error. I reported the issue here ropensci/RSelenium#230 but no response. maybe you could try upgrading the version of RSelenium used in DESCRIPTION? |
Okay, sure thing! |
|
||
coordPanel2 <- getNodeSet(info$newHtml, "//g[@class='geom5_path_viztwo']/g[@class='PANEL1']/path/@d") | ||
|
||
expect_false(identical(coordPanel1, coordPanel2)) |
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.
Does this test assume that getNodeSet
will always return the rendered points in the same order? I would prefer not to check it this way because even if the plot is not rendering any new points if getNodeSet returns them in a different order, this test might pass. Though if you are sure about the ordering, no changes needed. To me, the length of the retrieved nodes is a better indicator.
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.
I guess checking the length of retrieved nodes is a better indicator. I'll make changes and push as soon as possible.
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.
Hii @tdhock, @faizan-khan-iit,
I observed that the points are being rendered in order.
I checked and found this about the order.
So, the identical check is good to go. But if you say I can move to length check which will also work!
typically i the order does not matter then you can sort the
…On Sun, Mar 29, 2020 at 8:47 AM Himanshu Singh ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/testthat/test-renderer3-HitOrMissMonteCarlo.R
<#35 (comment)>:
> + y2_nodes <- getNodeSet(info$newHtml, ***@***.******@***.******@***.***")
+
+ expect_false(identical(x1_nodes, x2_nodes))
+ expect_false(identical(y1_nodes, y2_nodes))
+})
+
+test_that("Different points are rendered on Pi estimation plot", {
+ coordPanel1 <- getNodeSet(info$html, ***@***.******@***.******@***.***")
+
+ Sys.sleep(1.739)
+
+ info$newHtml <- getHTML()
+
+ coordPanel2 <- getNodeSet(info$newHtml, ***@***.******@***.******@***.***")
+
+ expect_false(identical(coordPanel1, coordPanel2))
I guess checking the length of retrieved nodes is a better indicator. I'll
make changes and push as soon as possible.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#35 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHDX4VXZK7KCIHQVD4O2SLRJ5UQPANCNFSM4LGUQCEQ>
.
|
@tdhock, Looks like the error didn't occur this time which occurred last time. But different error occurred this time which is in renderer5 and compiler. Have a look please? https://travis-ci.org/github/tdhock/animint2/builds/674748257
|
I've added HitOrMissMonteCarlo Test for Hard Test of
Animated interactive ggplots
I had to change the setImplicitTimeout method as it wasn't working and seems to be defunct.