Toby cookbook-axes

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍

[tdhock]

This branch translates some of Marianne's visual tests from the axes.R file in the add-r-cookbook-tests branch

https://github.com/ropensci/plotly/blob/add-r-cookbook-tests/tests/cookbook-test-suite/axes.R

to the format I need for adding them to the test table.

[tdhock]

TODO: add tests in R code for things like font color.

[tdhock]

Test table for this branch is

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/index.html

most tests are looking good except

http://ropensci.github.io/plotly-test-table/tables/fa8ab30229ca0491580b218f32b551738c7adfd2/cookbook-axes-coord-ylim.html

[chriddyp]

nice! should we just add the cookbook .R files all in one PR (so that we include them in our visual tests) and then do fixes on separate subsequent PRs?

[tdhock]

well I was thinking of proposing 1 PR per cookbook .R file, and adding fixes in the same PR along the way, so that master is always passing checks on travis.

[chriddyp]

OK, cool, that makes sense!

[tdhock]

Hey @cpsievert I did git pull origin master from this branch, and I was expecting that travis would run and redo the test table, but it does not seem to be running. Any idea why? If you know how to fix it then go ahead and push to this branch.

[cpsievert]

I think we just have to be patient. Travis is still building...

[cpsievert]

It's up now -- http://ropensci.github.io/plotly-test-table/

[tdhock]

awesome, but indeed there are some missing images in the current test table

http://ropensci.github.io/plotly-test-table/tables/00a5c8df16e2b19a6629c32ed42ef3a76d6fbac6/index.html

so I restarted the build via the travis web page like you suggested.

however I anticipate this will be an issue for almost all future builds, for example there is one missing image in your new branch's test table

http://ropensci.github.io/plotly-test-table/tables/f26e22467e4ad91b367bd5b15d9f68f304a7a4f9/density.html

From the plotly side it would be nice if @chriddyp could investigate and see why the plotly web site randomly returns an error when we ask for a png image.

From the R side I will attempt to modify the plotly-test-table code so that it checks for missing images and attempt to re-download them.

[cpsievert]

httr could help on the R side. Instead of these lines, you could do something like

library(httr)
g<- GET(plotly.png.url, write_disk(plotly.png.file))
warn_for_status(g)
# if we want to stop the build, use stop_for_status() instead

[tdhock]

thanks for the suggestion, I will look into that.

by the way after re-building the table it seems that the missing PNGs now are true negatives (the ggplotly function stops with an error for master, but works for this branch)

[chriddyp]

warn_for_status is great, the status code will be really helpful in debugging. my guess is that it is a timeout issue, but I'm not sure without seeing the status code. we could also put in a retry logic: if it failed, pause for 5 seconds, and then re-try.

[chriddyp]

Hey @tdhock and @cpsievert - what do you guys think about merging in all the cookbook .R files (without additional fixes) so that we can get a broader set of tests in our test-table before we merge other PRs like #178 and #167 (and risk degradation!)

[tdhock]

I still think it makes more sense to do one cookbook .R file at a time, to avoid having the tests on master failing. but if you think it is better to avoid regressions I can do that (and maybe if we want to avoid having the Travis master test fail, we can just add the image tests to the table without adding any expect_ calls in R code).

[tdhock]

by the way I think we can merge this branch right away, here is the test table

http://ropensci.github.io/plotly-test-table/tables/07af388fed4266ed8a87fb48ddb711aefcfa7655/index.html

[chriddyp]

could you add this as a github issue so that we can keep track of it?

[tdhock]

added #184

[chriddyp]

other than my comments, looks good to me 👍