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

test(artifacts): use tmp_path fixture instead of writing local files during tests #4521

Merged
merged 3 commits into from Nov 23, 2022

Conversation

moredatarequired
Copy link
Contributor

Description

Replace os.mkdir calls inside test functions with accepting the pytest built-in tmp_path fixture.

Checklist

  • Include reference to internal ticket "Fixes WB-NNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
  • Ensure PR title compliance with the conventional commits standards

@moredatarequired moredatarequired changed the title chore(artifacts): Use tmp_path fixture instead of writing local files during tests refactor(artifacts): Use tmp_path fixture instead of writing local files during tests Nov 23, 2022
@moredatarequired moredatarequired changed the title refactor(artifacts): Use tmp_path fixture instead of writing local files during tests test(artifacts): Use tmp_path fixture instead of writing local files during tests Nov 23, 2022
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #4521 (e95e6ae) into main (f6695ba) will increase coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4521      +/-   ##
==========================================
+ Coverage   83.08%   83.13%   +0.04%     
==========================================
  Files         259      259              
  Lines       32979    32979              
==========================================
+ Hits        27400    27416      +16     
+ Misses       5579     5563      -16     
Flag Coverage Δ
functest 56.97% <ø> (+0.05%) ⬆️
unittest 73.28% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wandb/integration/tensorboard/monkeypatch.py 88.88% <0.00%> (-2.47%) ⬇️
wandb/sdk/lib/git.py 79.01% <0.00%> (ø)
wandb/sdk/wandb_run.py 91.13% <0.00%> (+0.41%) ⬆️
wandb/sdk/service/server_sock.py 92.89% <0.00%> (+1.01%) ⬆️
wandb/filesync/dir_watcher.py 87.76% <0.00%> (+1.26%) ⬆️
wandb/filesync/step_prepare.py 94.93% <0.00%> (+1.26%) ⬆️
wandb/sdk/internal/system/system_info.py 91.55% <0.00%> (+3.24%) ⬆️

@moredatarequired moredatarequired merged commit 81f4965 into main Nov 23, 2022
@moredatarequired moredatarequired deleted the artifacts/use-tmp-dirs-for-tests branch November 23, 2022 22:36
andrewtruong pushed a commit that referenced this pull request Dec 2, 2022
…s during tests (#4521)

* Use tmp_path fixture instead of writing to the filesystem in tests

* Remove unused imports
@kptkin kptkin changed the title test(artifacts): Use tmp_path fixture instead of writing local files during tests test(artifacts): use tmp_path fixture instead of writing local files during tests Dec 6, 2022
@github-actions github-actions bot added cc-test and removed cc-test labels Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants