near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 615 forks source link

feat(forknet) - configure log config only in new_test #11812

Closed wacban closed 1 month ago

wacban commented 1 month ago

My goal is to figure out how to set the log level in forknet. Please consider this PR only as an RFC for what would be the best approach to solve this problem. I think in the perfect world the initial setup would happen only once e.g. in new_test or in reset. It may also be nice to add a command to adjust the log config but this can always be done with run-cmd so it's low pri.

I also refactored things a bit, sorry for mixing it with the real change. The real change is moving configure_log_config from start_neard to new_test, the rest should be a noop.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.77%. Comparing base (2d19c2a) to head (68ee7cf).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11812 +/- ## ========================================== + Coverage 71.76% 71.77% +0.01% ========================================== Files 796 796 Lines 163196 163196 Branches 163196 163196 ========================================== + Hits 117115 117133 +18 + Misses 41030 41020 -10 + Partials 5051 5043 -8 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <ø> (ø)` | | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <ø> (ø)` | | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.35% <ø> (ø)` | | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.83% <ø> (+0.02%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.38% <ø> (+<0.01%)` | :arrow_up: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.33% <ø> (-0.03%)` | :arrow_down: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `54.55% <ø> (-0.04%)` | :arrow_down: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.62% <ø> (ø)` | | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.42% <ø> (ø)` | | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.22% <ø> (-0.02%)` | :arrow_down: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11812/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wacban commented 1 month ago

I am going on PTO for a week so feel free to take over this PR if it's needed. If not I'll finish it up when I'm back.

marcelo-gonzalez commented 1 month ago

this looks good except we would need this change before submitting it:

diff --git a/pytest/tests/mocknet/helpers/neard_runner.py b/pytest/tests/mocknet/helpers/neard_runner.py
index f38eae92a..344de21f4 100644
--- a/pytest/tests/mocknet/helpers/neard_runner.py
+++ b/pytest/tests/mocknet/helpers/neard_runner.py
@@ -895,7 +895,7 @@ class NeardRunner:
         out_path = os.path.join(self.neard_logs_dir, self.neard_logs_file_name)
         with open(out_path, 'ab') as out:
             if self.is_traffic_generator():
-                self.get_start_traffic_generator_cmd(batch_interval_millis)
+                cmd = self.get_start_traffic_generator_cmd(batch_interval_millis)
             else:
                 cmd = self.get_start_cmd()

@@ -963,6 +963,8 @@ class NeardRunner:
             cmd.append('--config-path')
             cmd.append(self.target_near_home_path('mirror-config.json'))

+        return cmd
+
     # returns a bool that tells whether we should attempt a restart
     def on_neard_died(self):
         if self.is_traffic_generator():
marcelo-gonzalez commented 1 month ago

I am going on PTO for a week so feel free to take over this PR if it's needed. If not I'll finish it up when I'm back.

Oh i missed this comment yesterday. So yea i just pushed a commit w that diff above and ill submit it