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

Update tests (issue #76) #205

Merged
merged 41 commits into from
Jul 26, 2023
Merged

Update tests (issue #76) #205

merged 41 commits into from
Jul 26, 2023

Conversation

yolaj-nhs
Copy link
Contributor

@yolaj-nhs yolaj-nhs commented Jul 14, 2023

Improve the tests by doing the following:

  • Test the contents of the attribute inference attack in test_attribute_inference_attack.py
  • In test_generate_report.py - test for the contents of the generated file (not just whether they pass/fail)
  • In test_metrics.py - check the results of the calculation, not just pass/fail
  • Remove test_safekeras.old
  • The GenerateJSON module is tested inside other tests, which would make things very unclear if there was a failure. Refactor this so the tests are happening in the right place
  • test_lira_target should be cleaned after tests
  • Removing aisdc/safemodel/classifiers/dp_svc.py warning
  • Cleaning files that have been created in tests

Any suggestions of other improvements that could be made to the tests are welcome, and can also be added to the following ticket:

#76

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #205 (3a4e35b) into development (f9e92b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           development     #205   +/-   ##
============================================
  Coverage        99.55%   99.55%           
============================================
  Files               39       39           
  Lines             5137     5179   +42     
============================================
+ Hits              5114     5156   +42     
  Misses              23       23           
Files Changed Coverage Δ
tests/test_worst_case_attack.py 100.00% <ø> (ø)
aisdc/safemodel/classifiers/dp_svc.py 100.00% <100.00%> (ø)
tests/test_attack_report_formatter.py 100.00% <100.00%> (ø)
tests/test_attribute_inference_attack.py 100.00% <100.00%> (ø)
tests/test_failfast.py 100.00% <100.00%> (ø)
tests/test_lira_attack.py 100.00% <100.00%> (ø)
tests/test_metrics.py 99.17% <100.00%> (+0.08%) ⬆️
tests/test_multiple_attacks.py 100.00% <100.00%> (ø)
tests/test_safekeras2.py 100.00% <100.00%> (ø)
tests/test_safemodel.py 100.00% <100.00%> (ø)

@yolaj-nhs yolaj-nhs marked this pull request as ready for review July 17, 2023 14:29
@rpreen
Copy link
Contributor

rpreen commented Jul 17, 2023

This looks good.

While you're on this, could you also fix the annoying warning (which I know is not directly coming from the tests) that's been there for ages?

aisdc/safemodel/classifiers/dp_svc.py:137: RuntimeWarning: divide by zero encountered in double_scalars
    self.dpsvc_gamma = 1.0 / np.sqrt(

@rpreen rpreen self-requested a review July 25, 2023 11:25
rpreen
rpreen previously requested changes Jul 25, 2023
Copy link
Contributor

@rpreen rpreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has a few merge conflicts now that need to be addressed because of changes in other PRs.

Copy link
Contributor

@simonrnss simonrnss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great -- just added comments where I found hard-coded directory separators...

@simonrnss
Copy link
Contributor

Sorry - vague comments...I'd prefer not to hard-code /, but I'm not sure if the is.dir will work without it? So, feel free to leave them if it's an issue.

@yolaj-nhs yolaj-nhs requested a review from rpreen July 26, 2023 09:10
@yolaj-nhs
Copy link
Contributor Author

This PR has a few merge conflicts now that need to be addressed because of changes in other PRs.

Merge conflicts have been resolved

@yolaj-nhs yolaj-nhs dismissed rpreen’s stale review July 26, 2023 09:15

Merge conflicts have been resolved

@yolaj-nhs yolaj-nhs merged commit 2e730df into development Jul 26, 2023
4 checks passed
@yolaj-nhs yolaj-nhs deleted the enhance_tests branch July 26, 2023 09:21
@yolaj-nhs yolaj-nhs mentioned this pull request Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants