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

refactor(daa): remove global daa test mode #802

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Sep 29, 2023

Depends on #801

Motivation

This PR refactors the global daa TEST_MODE, allowing its simulator patch to be removed.

Acceptance Criteria

  • Remove the global daa.TEST_MODE and daa._set_test_mode(), moving it to an attribute in the DAA class. Also, update respective usages.
  • Create temporary DAA singleton to be used exclusively in the PeerId class. It should be removed in a future PR.
  • Remove _set_test_mode() simulator patch.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Sep 29, 2023
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from cf8df1d to 75f9964 Compare September 29, 2023 22:42
@glevco glevco marked this pull request as ready for review September 29, 2023 23:03
jansegre
jansegre previously approved these changes Oct 6, 2023
@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch from 09ee92f to c76fb12 Compare October 10, 2023 03:03
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from 75f9964 to fd22533 Compare October 10, 2023 03:09
@glevco glevco marked this pull request as draft October 10, 2023 21:09
@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch 2 times, most recently from d2f2993 to 22c4e78 Compare November 3, 2023 18:45
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from fd22533 to 88c0162 Compare November 3, 2023 18:52
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (2bcbc81) 85.11% compared to head (3926fa5) 85.12%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #802      +/-   ##
==========================================
+ Coverage   85.11%   85.12%   +0.01%     
==========================================
  Files         275      275              
  Lines       22514    22516       +2     
  Branches     3434     3434              
==========================================
+ Hits        19162    19167       +5     
+ Misses       2670     2667       -3     
  Partials      682      682              
Files Coverage Δ
hathor/daa.py 95.53% <100.00%> (-0.04%) ⬇️
hathor/p2p/peer_id.py 97.22% <100.00%> (+0.03%) ⬆️
hathor/simulator/simulator.py 94.47% <100.00%> (-0.04%) ⬇️
hathor/builder/cli_builder.py 74.87% <42.85%> (+0.12%) ⬆️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glevco glevco marked this pull request as ready for review November 3, 2023 20:10
@glevco glevco force-pushed the refactor/simulator-patches/daa-settings branch from 22c4e78 to 2bcbc81 Compare November 3, 2023 21:08
@glevco glevco force-pushed the refactor/simulator-patches/daa-test-mode branch from 88c0162 to 3926fa5 Compare November 3, 2023 21:09
Base automatically changed from refactor/simulator-patches/daa-settings to master November 3, 2023 21:11
@msbrogli msbrogli dismissed jansegre’s stale review November 3, 2023 21:11

The base branch was changed.

@glevco glevco merged commit 9150878 into master Nov 3, 2023
9 checks passed
@glevco glevco deleted the refactor/simulator-patches/daa-test-mode branch November 3, 2023 22:34
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants