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

Dev 887 #1078

Merged
merged 2 commits into from
Aug 8, 2023
Merged

Dev 887 #1078

merged 2 commits into from
Aug 8, 2023

Commits on Aug 8, 2023

  1. Fix 887 and improve code coverage of R/config.R from 28.21% to 100% (

    …#1043)
    
    * fix: add user supplied path to config for guess_where_config()
    
    - add helper try_user_config_location()
        - search for new config location to be set inside R/app_config.R
    - update guess_where_config() with a final guess using this helper
    - run styler::style_file("R/config.R", transformers =
      grkstyle::grk_style_transformers())
    
    * fix: logical scalar operation should use && instead of &
    
    * test: guess_where_config() works with path changes
    
    - path changes mean new dir and new filename so:
      - create a new dir for config
      - move config to that dir and rename
    
    => test correctly identifies changes (changing dir AND changing filename) in the yaml-config.
    
    attribute to @ALanguillaume with some minor tweaks
    
    Co-authored-by: ALanguillaume <[email protected]>
    
    * refactor: improve readability and logical of `try_user_config_location()`
    
    - encapsulate finding the relevant lines in the config file into "guess_lines_to_config_file()"
        - function allows for multi-line/grk-style app_sys()-calls (if file path to long and must be put into next line)
        - has clearer and commented logic to find the lines
    - document proper logic of function with comments
    - style with `grkstyle::grk_style_transformer`
    - update `tests/testthat/test-config.R` to incorporate above changes and move code coverage of that file to 63.16%
    
    * style: try_user_config_location() returns fs-paths for consistency
    
    - allows valid comparison with fs_path()'s inside guess_where_config() and output values from try_user_config_location()
    - fix a typo in a comment
    
    * refactor: improve readability and logic inside guess_where_config
    
    Why:
    
    - clearly outline the different cases: since now the user can add a golem-config, there can be conflicting cases between the default golem config and the user golem config (if e.g. the user forgets to delete the default)
    - update the tests to check these different cases
    - remark: test inside testthat should work without golem:: prefixes
    
    * test: add test for exotic corner cases of guess_where_config()
    
    - test that config file can be found if
       - we end up inside "inst/" (a corner case not tested before)
       - guess_where_config() is fed with argument values for 'path' and 'file' that do not work
    
    -> improves code coverage to 70.75%
    
    * feature: guess_where_config() fails a bit faster
    
    - remove last if-statement (which is almost a duplicate of the one before)
       -> this makes the guess_where_config() fail easier (i.e. return NULL) as other exotic corner cases should be handled more explicitly
    - remark: golem::pkg_path() will (almost) never produce an error but this may change so the try-construct within attempt() is kept
    
    - improve comments for corner case IV.B as internal developer note
    
    * refactor: encapsulate yesno() for clarity and easier testing
    
    * test: move coverage of R/config.R to 100%
    
    - notably: add package mockery to Suggests (no dependencies except for testthat and used for tests only)
    
    - add tests for non-interactive usage of guess_where_config(): check that error is thrown correctly
    
    - add tests for interactive usage of guess_where_config():
        - user says yes: test that necessary files are created
        - user says no: test that we return NULL
    
    - add tests for encapsulated ask_golem_creation_upon_config()
        - shallow test as non-interactive menu() calls (inside the yesno()) are forbidden
        - but this makes code coverage 100% :)
    
    * docs: update docs for get_current_config()
    
    - only exported function from R/config.R needs proper docs
    - besides more details on older usage and behavior:
        - add docs on how to set user supplied golem config
        - add code snippets
    
    ---------
    
    Co-authored-by: ALanguillaume <[email protected]>
    ilyaZar and ALanguillaume authored Aug 8, 2023
    Configuration menu
    Copy the full SHA
    ba26195 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    16db95f View commit details
    Browse the repository at this point in the history