prjemian / punx

Python Utilities for NeXus HDF5 files
https://prjemian.github.io/punx
5 stars 7 forks source link

improve code coverage of unit tests #184

Closed prjemian closed 2 years ago

prjemian commented 2 years ago

Via this effort, the API and documentation will be improved.

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 3bbebc3bd41a66d895146c7517f2cbd8bc2351e9 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 88933844af2597d66d5738661c38ea354b66b0ab into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

new alerts:

prjemian commented 2 years ago

GitHub rate limits big time problem now. Not certain the process is receiving the correct token.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+7.3%) to 95.209% when pulling 574213372de26ec2b169ee9373d9d291b9f56658 on 183-code-coverage into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 on main.

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 9d21c40dd70e4d220fc32b3eb3c62bc52817daaf into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

new alerts:

prjemian commented 2 years ago

some workflows succeeding, some not, all random failures due to APR rate limits now

coverage on this file up to 81% now (overall now 91%): https://coveralls.io/builds/45239813/source?filename=punx%2Fcache_manager.py

prjemian commented 2 years ago

Restarting the failed workflows. First one succeeded. Code coverage is up (91%) but now the tests are more fragile, depending on external conditions.

prjemian commented 2 years ago

still an API rate limit

Did not find environment variables GH_TOKEN or GITHUB_TOKEN which provide the GitHub API token necessary to download resources from GitHub through its API.  You may experience restrictions on the amount of content that can be downloaded over a short interval (such as an hour or so).

which means this is not working: https://github.com/prjemian/punx/blob/3509d29acd3ee38d06d1def6a3022ff010eb9ea2/.github/workflows/unit-tests-pytest.yml#L53-L54

prjemian commented 2 years ago

From another repo, proved that environment variables can be found (whew). Some of the tests remove the token(s) from the environment (found locally) so need to save (before) and restore (after) for each use. Make the github token into a test fixture.

prjemian commented 2 years ago

coverage of punx.cache_manager now at 94%. Next ...

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 1e1feefc09c305699941cc4cd4f9b89bce0c4d44 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging b7b090f07bb780499d1831888b83725aa6560de0 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

prjemian commented 2 years ago

Unit tests are still interrupted (during periods of frequent workflow runs) by the GitHub API Rate Limit, making success of the test process appear to be fragile. Can the use of the GitHub API be reduced or eliminated entirely? Most of this code relies on use of a file set (directory with NeXus definitions content from a release, tag, branch, or commit in the NeXus definitions repository) that is stored locally. Any of these file sets could be downloaded (without need of the API) using a URL:

    file_set_name = "v2020.10" or "main" or "183-code-coverage" or "Schema-3.4" or "a4fd52d"
    compressed_format = "zip" or "tar.gz"
    url = f"https://github.com/prjemian/punx/archive/{file_set_name}.{compressed_format}"

Only the update subcommand interacts with GitHub. With update, there are two possibilities for download:

  1. install : download and install a file set that does not exist locally
  2. replace : download and replace a file set that exists locally

The GitHub API is useful in the latter case to determine whether or not a new version exists. Practically, only a branch file set (such as main) could have a new version and thus be updated. At the expense of excessive bandwidth to the end user, the replace would always install the latest commit matching the file set name.

Simpler? Likely would lose the identification of the type (commit, tag, branch, release). Not a major problem.

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 749deb7ac0f20d00ba6d8cd7be2ab79f18599808 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging f373b6253428f91016e9fef05b2b05d8efc6f471 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 759ec7bea6edac639554fb175c9139177e6a4a0f into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging fea5482e0d12ad0a558c4ffc61507a41e4be6082 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging eed9341effb78f32c81449d17de6f48f2c0da77c into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 25adbebb442fd65fcfed663578b45c32b88ac0dc into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 28aca1b7d13edc19ab43f98895d549f970c9df38 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts:

prjemian commented 2 years ago

JavaScript test fails from LGTM are not relevant to this project.

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 574213372de26ec2b169ee9373d9d291b9f56658 into 6e3a0e7ab8bb9b0993d05e35e163b66eaa552f20 - view on LGTM.com

fixed alerts: