trying to fix-Init should refuse if there are existing results #560

muhd360 · 2024-10-28T07:23:31Z

issue number -#417
changes:-added force flag
added tests

abingham · 2024-10-28T07:30:41Z

src/cosmic_ray/cli.py

@@ -96,7 +97,11 @@ def init(config_file, session_file):
            log.info(" - %s: %s", directory, ", ".join(sorted(files)))

    with use_db(session_file) as database:
-        cosmic_ray.commands.init(modules, database, operators_cfg)
+        if database.num_results>0:


I think this logic is incorrect. If database.num_results is greater than zero, it will always exit, event if force is True. What we want is for initialization to occur if force is True, no matter what the state of the existing database is.

muhd360 · 2024-10-28T07:37:03Z

Ok will try

…

On Mon, 28 Oct, 2024, 13:01 Austin Bingham, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cosmic_ray/cli.py <#560 (comment)> : > @@ -96,7 +97,11 @@ def init(config_file, session_file): log.info(" - %s: %s", directory, ", ".join(sorted(files))) with use_db(session_file) as database: - cosmic_ray.commands.init(modules, database, operators_cfg) + if database.num_results>0: I think this logic is incorrect. If database.num_results is greater than zero, it will always exit, event if force is True. What we want is for initialization to occur if force is True, no matter what the state of the existing database is. — Reply to this email directly, view it on GitHub <#560 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A6BG3WUQYV6IVZPX2NJOPETZ5XK2PAVCNFSM6AAAAABQWYLI52VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOJYGE2DMMZWGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

abingham · 2024-10-28T07:37:27Z

tests/unittests/test_command_line_processing.py

@@ -59,11 +60,24 @@ def test_non_existent_session_file_returns_EX_NOINPUT(local_unittest_config):
    assert cosmic_ray.cli.main(["exec", str(local_unittest_config), "foo.session"]) == ExitCode.NO_INPUT


-def test_non_existent_config_file_returns_EX_NOINPUT(session, local_unittest_config):
-    cosmic_ray.cli.main(["init", local_unittest_config, str(session)])
+def test_non_existent_config_file_returns_EX_NOINPUT(session, local_unittest_config,force):


This test (and all tests which take a force parameter) don't work because you don't have a fixture providing the value for force. In all cases, I don't think the tests actually need to use a fixture for force. You can simply pass the correct value - True or False - into cosmic_ray.cli.main.

abingham · 2024-10-28T07:38:51Z

tests/unittests/test_command_line_processing.py

+def test_init_with_existing_results_no_force(session, local_unittest_config,force):
+    """Test that init exits without reinitializing when results exist and force=False"""
+    with use_db(session) as database:
+        database.num_results = 1  # Simulate existing results


You can't just set num_results like this. The WorkDB class won't allow it.

You clearly haven't run these tests because none of them run. Please don't resubmit until the tests actually run.

sorry i commited the wrong branch will fix the changes

muhd360 · 2024-10-29T04:28:59Z

hi @abingham for the case when force is false what will the test case reflect is creating a mockdb the right kind of approach???

muhd360 · 2024-10-30T03:56:58Z

@abingham ???

abingham · 2024-10-31T07:54:21Z

hi @abingham for the case when force is false what will the test case reflect is creating a mockdb the right kind of approach???

A test for the force == False case would not require a mock database. You could create a normal workdb, get some results into it, and then verify that running init with force == False aborts and does not re-init the database, i.e. it would leave the database unchanged.

muhd360 · 2024-10-31T12:19:02Z

ok will do

muhd360 · 2024-10-31T19:31:24Z

hi @abingham both the tests are still failing can u pls check them once and guide me to the necessary steps so that i can fix my mistake i cant seem to figure it out

abingham · 2024-11-01T06:35:28Z

tests/conftest.py

@@ -2,7 +2,9 @@

 import pytest

-
+@pytest.fixture


This fixture doesn't seem to be doing anything. It should be removed.

abingham · 2024-11-01T06:37:39Z

tests/unittests/test_command_line_processing.py

+    """Test that init exits without reinitializing when results exist and force=False"""
+    # Sample WorkItem creation
+    mutation_spec = MutationSpec(
+        module_path="src/example/test.py",


Pass a Path object here, not a string. I know that MutationSpec will convert it, but I'd like to get away from depending on that.

abingham · 2024-11-01T06:38:25Z

tests/unittests/test_command_line_processing.py

+        )
+    ]
+
+    db=WorkDB(session,1)


Use WorkDB.Mode.create here instead of just 1.

Also, it would be more idiomatic to use the use_db context manager rather than opening the WorkDB directly like this.

abingham · 2024-11-01T06:38:59Z

tests/unittests/test_command_line_processing.py

+
+
+    result = cosmic_ray.cli.main(["init", local_unittest_config, str(session)])
+    db=WorkDB(session,2)


Use WorkDB.Mode.open here, not just 2. And use use_db.

abingham · 2024-11-01T06:39:59Z

tests/unittests/test_command_line_processing.py

+
+
+
+def test_init_with_existing_results_force(session, local_unittest_config,force):


As mentioned earlier, you don't need (and aren't really using) the force fixture here. Remove it.

abingham · 2024-11-01T06:42:01Z

tests/unittests/test_command_line_processing.py

+
+
+def test_init_with_existing_results_force(session, local_unittest_config,force):
+    """Test that reinitialization occurs when force=True"""


This isn't testing what it claims to test. You're not re-initializing the database, just initializing it, so passing --force doesn't do anything special in this case. You need to:

Create the WorkDB

Put some results into it and verify that it has the results

Re-initialize it with --force

Assert that it now contains no results.

abingham · 2024-11-01T06:58:20Z

tests/unittests/test_command_line_processing.py

@@ -64,6 +66,56 @@ def test_non_existent_config_file_returns_EX_NOINPUT(session, local_unittest_con
    assert cosmic_ray.cli.main(["exec", "no-such-file", str(session)]) == ExitCode.CONFIG


+
+def test_init_with_existing_results_no_force(session, local_unittest_config):


You'll probably need to use the lobotomize fixture here to prevent CR from trying to actually scan the non-existent module references in local_unittest_config.

abingham · 2024-11-01T07:02:50Z

tests/unittests/test_command_line_processing.py

+    result = cosmic_ray.cli.main(["init", local_unittest_config, str(session)])
+    db=WorkDB(session,2)
+    final_count = db.num_results
+    assert final_count == initial_count  # Confirm work items are unchanged


You seem to be comparing the wrong things here. final_count is the number of results, i.e. the number of work items which have actually been executed. initial_count is the simply the total number of work items, whether or not they have results attached to them. You need to actually get some results into the workdb for this test to make any sense, and even then you need to make sure you compare the number of results before and after your init call.

muhd360 · 2024-11-01T11:02:28Z

thank u so much.....will do the necesarry changes.thx once again

first_try

6f5d807

abingham reviewed Oct 28, 2024

View reviewed changes

abingham requested changes Oct 28, 2024

View reviewed changes

second try

766d26c

abingham reviewed Nov 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

trying to fix-Init should refuse if there are existing results #560

trying to fix-Init should refuse if there are existing results #560

muhd360 commented Oct 28, 2024

abingham Oct 28, 2024

muhd360 commented Oct 28, 2024 via email

abingham Oct 28, 2024

abingham Oct 28, 2024

muhd360 Oct 28, 2024

muhd360 commented Oct 29, 2024

muhd360 commented Oct 30, 2024

abingham commented Oct 31, 2024

muhd360 commented Oct 31, 2024

muhd360 commented Oct 31, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

abingham Nov 1, 2024

muhd360 commented Nov 1, 2024



		result = cosmic_ray.cli.main(["init", local_unittest_config, str(session)])
		db=WorkDB(session,2)




		def test_init_with_existing_results_force(session, local_unittest_config,force):



		def test_init_with_existing_results_force(session, local_unittest_config,force):
		"""Test that reinitialization occurs when force=True"""

		@@ -64,6 +66,56 @@ def test_non_existent_config_file_returns_EX_NOINPUT(session, local_unittest_con
		assert cosmic_ray.cli.main(["exec", "no-such-file", str(session)]) == ExitCode.CONFIG



		def test_init_with_existing_results_no_force(session, local_unittest_config):

trying to fix-Init should refuse if there are existing results #560

Are you sure you want to change the base?

trying to fix-Init should refuse if there are existing results #560

Conversation

muhd360 commented Oct 28, 2024

Choose a reason for hiding this comment

muhd360 commented Oct 28, 2024 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhd360 commented Oct 29, 2024

muhd360 commented Oct 30, 2024

abingham commented Oct 31, 2024

muhd360 commented Oct 31, 2024

muhd360 commented Oct 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muhd360 commented Nov 1, 2024