Dem module tests

[meiertgrootes]

limited test suite framework for valley.R

[meiertgrootes]

@fnattino This PR sets up limited tests for the functionalities in valley.R. Would be good to discuss what else to add. This will also require (some) realworld test data to be added to the package. Please advise on this. Some paths are currently place holders.

[fnattino]

Thanks a lot for starting this @meiertgrootes! Once #42 is merged, I can add some test data to the package (#49), which we can use for testing.

[fnattino]

Suggested change

	})
	})

[fnattino]

@cforgaci #51 is merged, we can move on with the tests!

[cforgaci]

Thanks, @meiertgrootes! I made a few additional commits:

• merged the package data from Add packaged DEM data #51
• replaced placeholders with package data in the tests
• created test data for the last test

A few things left to do:

1. We should mock the API calls in the first two tests and allow them to run on CI. I will try this with the mockery package.

2. When dropping the skip_on_ci() from the last test, I've encountered some issues, will try to fix them and get back with an update.

[cforgaci]

A few things left to do:

1. We should mock the API calls in the first two tests and allow them to run on CI. I will try this with the mockery package.

This is done.

2. When dropping the skip_on_ci() from the last test, I've encountered some issues, will try to fix them and get back with an update.

@fnattino, I am having issues with accessing tests/testthat/fixtures/expected_valley.gpkg in the last test. All tests pass when I run devtools::test() but R CMD check fails with Error: Cannot open "fixtures/expected_valley.gpkg"; The file doesn't seem to exist. In the current option, I try using tests/testthat/setup.R to make a copy of the fixtures directory to the root of the package installed temprarily by R CMD check, but this does not seem to work either.

I also tried without setup.R and with the full path on line 61, same error:

expected_valley <- sf::st_read("tests/testthat/fixtures/expected_valley.gpkg",
                                quiet = TRUE) |>
    sf::st_as_sfc()

I know you tried once using test data under the tests directory. Any ideas why I am encountering this issue?

[cforgaci]

@fnattino thanks for the review! After implementing the suggested changes, I still see Error: Cannot open "testdata/expected_valley.gpkg"; The file doesn't seem to exist. in R-CMD-check.

[fnattino]

So annoying.. By googleing a bit I found that in this package very simple relative links seems to work: https://github.com/jeroen/openssl/blob/master/tests/testthat/test_cert.R

So can we try to do:

  expected_valley <- sf::st_read("./testdata/expected_valley.gpkg",
                                 quiet = TRUE) |>
    sf::st_as_sfc()

Locally tests pass..

[cforgaci]

So annoying.. By googleing a bit I found that in this package very simple relative links seems to work: https://github.com/jeroen/openssl/blob/master/tests/testthat/test_cert.R

So can we try to do:

  expected_valley <- sf::st_read("./testdata/expected_valley.gpkg",
                                 quiet = TRUE) |>
    sf::st_as_sfc()

Locally tests pass..

I tried this too, and I get the same results: locally tests pass and CI check fails.

[fnattino]

That package also has the --install-tests option as install argument in the .Rproj file:
https://github.com/jeroen/openssl/blob/597003e7aa95106cd34ff82c431e551a1e1e2978/openssl.Rproj#L20C55-L20C70

This should add the tests (and testdata) to the build, maybe is this the solution? If this does not work as well we might consider whether we actually want to remove the data..

[cforgaci]

@fnattino the testdata issue is solved, but I have one last issue I cannot solve. The last test did not pass R-CMD-check on Wihdows due to a precision issue and after a couple of attempts I still cannot figure out how to solve that. Any thoughts? I remember you gave a solution to a similar issue, maybe her in the same PR.

[fnattino]

The following should allow for some tolerance. The "exact" means that the two features are used as-they-are, so vertices need to be ordered in the same way. The "par" argument actually set the tolerance for equality:

sf::st_equals_exact(geometry1, geometry2, par =1.e-4, sparse = FALSE)

[cforgaci]

sf::st_equals_exact(geometry1, geometry2, par =1.e-4, sparse = FALSE)

@fnattino that was it. I had to adjust the precision of the two objects. All checks seem to be passing. If you agree, we can merge this now.

[fnattino]

Thanks for all the effort with this @cforgaci - this was quite more painful than anticipated.. go ahead and merge for my side!

[cforgaci]

Thanks for all the effort with this @cforgaci - this was quite more painful than anticipated.. go ahead and merge for my side!

Painful is a good word :) Thanks @fnattino!