From 65042c63fdd2e6791704f7a4cf3aa63428a82ef6 Mon Sep 17 00:00:00 2001 From: TeachMeTW Date: Mon, 16 Dec 2024 11:55:15 -0800 Subject: [PATCH 1/3] Fix file handling to prevent ValueError and terminal crashes - Replaced manual file open/close with `with` statement to ensure proper resource management. - Removed redundant file read operation after the file was closed. - Resolved `ValueError: I/O operation on closed file` and addressed terminal crashing issue during execution. --- .../net/ext_service/transit_matching/match_stops.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/emission/net/ext_service/transit_matching/match_stops.py b/emission/net/ext_service/transit_matching/match_stops.py index afa5d6abd..3e6deb1c3 100644 --- a/emission/net/ext_service/transit_matching/match_stops.py +++ b/emission/net/ext_service/transit_matching/match_stops.py @@ -15,12 +15,12 @@ url = "https://lz4.overpass-api.de/" try: - query_file = open('conf/net/ext_service/overpass_transit_stops_query_template') -except: + with open('conf/net/ext_service/overpass_transit_stops_query_template', 'r', encoding='UTF-8') as query_file: + query_string = "".join(query_file.readlines()) +except FileNotFoundError: print("transit stops query not configured, falling back to default") - query_file = open('conf/net/ext_service/overpass_transit_stops_query_template.sample') - -query_string = "".join(query_file.readlines()) + with open('conf/net/ext_service/overpass_transit_stops_query_template.sample', 'r', encoding='UTF-8') as query_file: + query_string = "".join(query_file.readlines()) RETRY = -1 From 7f2e460f38169aee1c7adfe808b0ded87e6bf0bd Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Thu, 19 Dec 2024 01:22:31 -0500 Subject: [PATCH 2/3] =?UTF-8?q?clear=20AND=20reload=20analysis=20config=20?= =?UTF-8?q?on=20tests=E2=80=99=20tearDown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are several tests that call `etc.set_analysis_config` in their `setUp()` to set config options in certain testing scenarios. This creates a config file, and in tearDown of each test the config file is deleted via os.remove. However, just deleting the file is not enough! When emission.analysis.config is intialized, we read the config and cache it in a variable `config_data` https://github.com/e-mission/e-mission-server/blob/4034533bfc95dbd7c976146341acd3527f9bc7c9/emission/analysis/config.py#L19 When get_config() is called while the pipeline is running, it returns the value of `config_data`, which can be out of sync with the contents of the config file if the config file was added/removed/modified and `reload_config()` was not called. So instead of just calling `os.remove()` these tests also need to call `reload_config()` in their `tearDown()`. Added this by way of a new function in `emission.tests.common` Also, I made the config file pathnames into constants for tidiness and to eliminate the risk of typo-related bugs --- emission/analysis/config.py | 10 +++++--- .../intakeTests/TestFilterAccuracy.py | 5 ++-- .../intakeTests/TestPipelineRealData.py | 9 +++---- .../intakeTests/TestSectionSegmentation.py | 6 ++--- .../intakeTests/TestTripSegmentation.py | 5 ++-- emission/tests/common.py | 24 +++++++++++-------- .../netTests/TestMetricsCleanedSections.py | 5 ++-- .../netTests/TestMetricsConfirmedTrips.py | 5 ++-- 8 files changed, 34 insertions(+), 35 deletions(-) diff --git a/emission/analysis/config.py b/emission/analysis/config.py index d484e5354..8dc514dcf 100644 --- a/emission/analysis/config.py +++ b/emission/analysis/config.py @@ -1,17 +1,21 @@ import json import os +ANALYSIS_CONF_PATH = "conf/analysis/debug.conf.json" +ANALYSIS_CONF_PROD_PATH = "conf/analysis/debug.conf.prod.json" +ANALYSIS_CONF_DEV_PATH = "conf/analysis/debug.conf.dev.json" + def get_config_data(): try: print("Trying to open debug.conf.json") - config_file = open('conf/analysis/debug.conf.json') + config_file = open(ANALYSIS_CONF_PATH) except: if os.getenv("PROD_STAGE") == "TRUE": print("In production environment, config not overridden, using default production debug.conf") - config_file = open('conf/analysis/debug.conf.prod.json') + config_file = open(ANALYSIS_CONF_PROD_PATH) else: print("analysis.debug.conf.json not configured, falling back to sample, default configuration") - config_file = open('conf/analysis/debug.conf.dev.json') + config_file = open(ANALYSIS_CONF_DEV_PATH) ret_val = json.load(config_file) config_file.close() return ret_val diff --git a/emission/tests/analysisTests/intakeTests/TestFilterAccuracy.py b/emission/tests/analysisTests/intakeTests/TestFilterAccuracy.py index 55ae19d90..e51e881a1 100644 --- a/emission/tests/analysisTests/intakeTests/TestFilterAccuracy.py +++ b/emission/tests/analysisTests/intakeTests/TestFilterAccuracy.py @@ -34,15 +34,14 @@ def setUp(self): import emission.core.get_database as edb import uuid - self.analysis_conf_path = \ - etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) + etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) self.testUUID = None def tearDown(self): import emission.core.get_database as edb edb.get_timeseries_db().delete_many({"user_id": self.testUUID}) edb.get_pipeline_state_db().delete_many({"user_id": self.testUUID}) - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() def checkSuccessfulRun(self): pipelineState = edb.get_pipeline_state_db().find_one({"user_id": self.testUUID, diff --git a/emission/tests/analysisTests/intakeTests/TestPipelineRealData.py b/emission/tests/analysisTests/intakeTests/TestPipelineRealData.py index f01cdc042..35808bbcf 100644 --- a/emission/tests/analysisTests/intakeTests/TestPipelineRealData.py +++ b/emission/tests/analysisTests/intakeTests/TestPipelineRealData.py @@ -60,8 +60,7 @@ class TestPipelineRealData(unittest.TestCase): def setUp(self): # Thanks to M&J for the number! np.random.seed(61297777) - self.analysis_conf_path = \ - etc.set_analysis_config("analysis.result.section.key", "analysis/cleaned_section") + etc.set_analysis_config("analysis.result.section.key", "analysis/cleaned_section") logging.info("setUp complete") def tearDown(self): @@ -76,8 +75,7 @@ def tearDown(self): # to determine whether to switch to a new implementation if not hasattr(self, "evaluation") or not self.evaluation: self.clearRelatedDb() - if hasattr(self, "analysis_conf_path"): - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() if hasattr(self, "seed_mode_path"): os.remove(self.seed_mode_path) logging.info("tearDown complete") @@ -744,8 +742,7 @@ def testJackUntrackedTimeMar12(self): def testJackUntrackedTimeMar12InferredSections(self): # Setup to use the inferred sections - self.analysis_conf_path = \ - etc.set_analysis_config("analysis.result.section.key", "analysis/inferred_section") + etc.set_analysis_config("analysis.result.section.key", "analysis/inferred_section") # along with the proper random seed self.seed_mode_path = etc.copy_dummy_seed_for_inference() dataFile = "emission/tests/data/real_examples/jack_untracked_time_2023-03-12" diff --git a/emission/tests/analysisTests/intakeTests/TestSectionSegmentation.py b/emission/tests/analysisTests/intakeTests/TestSectionSegmentation.py index df37a3c77..4162962fd 100644 --- a/emission/tests/analysisTests/intakeTests/TestSectionSegmentation.py +++ b/emission/tests/analysisTests/intakeTests/TestSectionSegmentation.py @@ -41,8 +41,7 @@ class TestSectionSegmentation(unittest.TestCase): def setUp(self): - self.analysis_conf_path = \ - etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) + etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) etc.setupRealExample(self, "emission/tests/data/real_examples/shankari_2015-aug-27") self.androidUUID = self.testUUID @@ -58,8 +57,7 @@ def setUp(self): def tearDown(self): if not hasattr(self, "evaluation") or not self.evaluation: self.clearRelatedDb() - if hasattr(self, "analysis_conf_path"): - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() def clearRelatedDb(self): edb.get_timeseries_db().delete_many({"user_id": self.androidUUID}) diff --git a/emission/tests/analysisTests/intakeTests/TestTripSegmentation.py b/emission/tests/analysisTests/intakeTests/TestTripSegmentation.py index 0cc469fea..37af7bea6 100644 --- a/emission/tests/analysisTests/intakeTests/TestTripSegmentation.py +++ b/emission/tests/analysisTests/intakeTests/TestTripSegmentation.py @@ -36,8 +36,7 @@ class TestTripSegmentation(unittest.TestCase): def setUp(self): - self.analysis_conf_path = \ - etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) + etc.set_analysis_config("intake.cleaning.filter_accuracy.enable", True) etc.setupRealExample(self, "emission/tests/data/real_examples/shankari_2015-aug-27") self.androidUUID = self.testUUID @@ -51,7 +50,7 @@ def setUp(self): logging.debug("androidUUID = %s, iosUUID = %s" % (self.androidUUID, self.iosUUID)) def tearDown(self): - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() edb.get_timeseries_db().delete_many({"user_id": self.androidUUID}) edb.get_timeseries_db().delete_many({"user_id": self.iosUUID}) edb.get_pipeline_state_db().delete_many({"user_id": self.androidUUID}) diff --git a/emission/tests/common.py b/emission/tests/common.py index baae6053c..76e745555 100644 --- a/emission/tests/common.py +++ b/emission/tests/common.py @@ -263,25 +263,29 @@ def createDummyRequestEnviron(self, addl_headers, request_body): return test_environ def set_analysis_config(key, value): + """ + Tests that call this in their setUp must call clear_analysis_config in their tearDown + """ import emission.analysis.config as eac import shutil - analysis_conf_path = "conf/analysis/debug.conf.json" - shutil.copyfile("conf/analysis/debug.conf.dev.json", - analysis_conf_path) - with open(analysis_conf_path) as fd: + shutil.copyfile(eac.ANALYSIS_CONF_DEV_PATH, eac.ANALYSIS_CONF_PATH) + with open(eac.ANALYSIS_CONF_PATH) as fd: curr_config = json.load(fd) curr_config[key] = value - with open(analysis_conf_path, "w") as fd: + with open(eac.ANALYSIS_CONF_PATH, "w") as fd: json.dump(curr_config, fd, indent=4) - logging.debug("Finished setting up %s" % analysis_conf_path) - with open(analysis_conf_path) as fd: + logging.debug("Finished setting up %s" % eac.ANALYSIS_CONF_PATH) + with open(eac.ANALYSIS_CONF_PATH) as fd: logging.debug("Current values are %s" % json.load(fd)) eac.reload_config() - - # Return this so that we can delete it in the teardown - return analysis_conf_path + +def clear_analysis_config(): + import emission.analysis.config as eac + if os.path.exists(eac.ANALYSIS_CONF_PATH): + os.remove(eac.ANALYSIS_CONF_PATH) + eac.reload_config() def copy_dummy_seed_for_inference(): import shutil diff --git a/emission/tests/netTests/TestMetricsCleanedSections.py b/emission/tests/netTests/TestMetricsCleanedSections.py index 24eac37a6..66f4f08e6 100644 --- a/emission/tests/netTests/TestMetricsCleanedSections.py +++ b/emission/tests/netTests/TestMetricsCleanedSections.py @@ -23,8 +23,7 @@ class TestMetrics(unittest.TestCase): def setUp(self): - self.analysis_conf_path = \ - etc.set_analysis_config("analysis.result.section.key", "analysis/cleaned_section") + etc.set_analysis_config("analysis.result.section.key", "analysis/cleaned_section") etc.setupRealExample(self, "emission/tests/data/real_examples/shankari_2015-aug-21") self.testUUID1 = self.testUUID @@ -41,7 +40,7 @@ def setUp(self): def tearDown(self): self.clearRelatedDb() - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() def clearRelatedDb(self): edb.get_timeseries_db().delete_many({"user_id": self.testUUID}) diff --git a/emission/tests/netTests/TestMetricsConfirmedTrips.py b/emission/tests/netTests/TestMetricsConfirmedTrips.py index 27a91dda0..444c5ed60 100644 --- a/emission/tests/netTests/TestMetricsConfirmedTrips.py +++ b/emission/tests/netTests/TestMetricsConfirmedTrips.py @@ -17,8 +17,7 @@ class TestMetrics(unittest.TestCase): def setUp(self): - self.analysis_conf_path = \ - etc.set_analysis_config("analysis.result.section.key", "analysis/confirmed_trip") + etc.set_analysis_config("analysis.result.section.key", "analysis/confirmed_trip") self._loadDataFileAndInputs("emission/tests/data/real_examples/shankari_2016-06-20") self.testUUID1 = self.testUUID self._loadDataFileAndInputs("emission/tests/data/real_examples/shankari_2016-06-21") @@ -39,7 +38,7 @@ def _loadDataFileAndInputs(self, dataFile): def tearDown(self): self.clearRelatedDb() - os.remove(self.analysis_conf_path) + etc.clear_analysis_config() def clearRelatedDb(self): edb.get_timeseries_db().delete_many({"user_id": self.testUUID1}) From 09a27469d8a119c8d5465f691649309407f433c3 Mon Sep 17 00:00:00 2001 From: Jack Greenlee Date: Thu, 19 Dec 2024 23:05:02 -0500 Subject: [PATCH 3/3] fix EnvironmentLocationNotFound error during test-with-manual-install "Teardown the test environment" has been consistently failing. ``` Run source setup/teardown_tests.sh Removing environment from CondaError: Run 'conda init' before 'conda deactivate' EnvironmentLocationNotFound: Not a conda environment: /usr/share/miniconda/envs/emissiontest Error: Process completed with exit code 1. ``` I noticed that test-with-manual-install.yml follows a similar teardown procedure (with 'emission' as the environment instead of 'emissiontest') I saw that test-with-manual-install.yml invokes activates_conda.sh again right before calling the teardown script --- .github/workflows/test-with-manual-install.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-with-manual-install.yml b/.github/workflows/test-with-manual-install.yml index 4a81eb000..bfb8b530b 100644 --- a/.github/workflows/test-with-manual-install.yml +++ b/.github/workflows/test-with-manual-install.yml @@ -70,4 +70,6 @@ jobs: - name: Teardown the test environment shell: bash -l {0} - run: source setup/teardown_tests.sh + run: | + source setup/activate_conda.sh + source setup/teardown_tests.sh