Menu
▾
▴
[Opensync-commits] r5649 - in trunk: opensync/engine tests tests/engine-tests tests/mock-plugin
|
From: <svn...@op...> - 2009-05-28 14:48:16
|
Author: dgollub Date: Thu May 28 16:48:03 2009 New Revision: 5649 URL: http://www.opensync.org/changeset/5649 Log: Fix for mapping_engine_same_similar_conflict which was randomly failing. Sometimes I see the mapping_engine_same_similar_conflict test fail on my system. I tracked the issue down to the order in which the mock_sync plugin reports changes in mock_report_dir. If entryA is reported before entryB then the test passes. If entryB is reported first then the test fails. I have 2 patches. The first one modifies mock_sync to sort the changes by file name before reporting them. It also adds a test which takes advantage of that to reliably reproduce this issue. The second patch tries to address the root cause of the issue. I'm not completely familiar with the opensync internals so it may need some work. It works for me and doesn't break any of the other tests. Patch by marka fixes #1123 Modified: trunk/opensync/engine/opensync_obj_engine.c trunk/tests/CMakeLists.txt trunk/tests/engine-tests/check_mapping_engine.c trunk/tests/mock-plugin/mock_sync.c Modified: trunk/opensync/engine/opensync_obj_engine.c ============================================================================== --- trunk/opensync/engine/opensync_obj_engine.c Thu May 28 16:19:03 2009 (r5648) +++ trunk/opensync/engine/opensync_obj_engine.c Thu May 28 16:48:03 2009 (r5649) @@ -208,47 +208,44 @@ osync_bool found_similar = FALSE; OSyncConvCmpResult result = OSYNC_CONV_DATA_MISMATCH; osync_trace(TRACE_ENTRY, "%s(%p, %p, %p, %p)", __func__, mapping_engines, change, sinkengine, mapping_engine); - - for (m = mapping_engines; m; m = m->next) { - *mapping_engine = m->data; - + + for (m=mapping_engines; m && (result != OSYNC_CONV_DATA_SAME); m=m->next) { + OSyncMappingEngine *tmp_mapping_engine = m->data; + /* Go through the already existing mapping entries. We only consider mappings * which dont have a entry on our side and where the data comparsion does not * return MISMATCH */ - for (e = (*mapping_engine)->entries; e; e = e->next) { + for (e = tmp_mapping_engine->entries; e; e = e->next) { OSyncMappingEntryEngine *entry_engine = e->data; OSyncChange *mapping_change = NULL; + OSyncConvCmpResult tmp_result; + /* if the mapping already has a entry on our side, its not worth looking */ - if (entry_engine->sink_engine == sinkengine) { - if (!found_similar) - *mapping_engine = NULL; + if (entry_engine->sink_engine == sinkengine) + continue; - break; - } - mapping_change = osync_entry_engine_get_change(entry_engine); if (!mapping_change) continue; - - result = osync_change_compare(mapping_change, change); - if (result == OSYNC_CONV_DATA_MISMATCH) { - if (!found_similar) - *mapping_engine = NULL; - } else if (result == OSYNC_CONV_DATA_SAME) { + + tmp_result = osync_change_compare(mapping_change, change); + if(tmp_result == OSYNC_CONV_DATA_SAME) { + /* SAME is the best we can get */ + result = OSYNC_CONV_DATA_SAME; + *mapping_engine = tmp_mapping_engine; break; - } else if (result == OSYNC_CONV_DATA_SIMILAR) { - found_similar = TRUE; + } else if((tmp_result == OSYNC_CONV_DATA_SIMILAR) && + (result == OSYNC_CONV_DATA_MISMATCH)) + { + /* SIMILAR is better than MISMATCH */ + result = OSYNC_CONV_DATA_SIMILAR; + *mapping_engine = tmp_mapping_engine; } } - - if (*mapping_engine) { - osync_trace(TRACE_EXIT, "%s: Found %p", __func__, *mapping_engine); - return result; - } } - - osync_trace(TRACE_EXIT, "%s: Mismatch", __func__); - return OSYNC_CONV_DATA_MISMATCH; + + osync_trace(TRACE_EXIT, "%s: %d, %p", __func__, (int)result, *mapping_engine); + return result; } osync_bool osync_obj_engine_map_changes(OSyncObjEngine *engine, OSyncError **error) Modified: trunk/tests/CMakeLists.txt ============================================================================== --- trunk/tests/CMakeLists.txt Thu May 28 16:19:03 2009 (r5648) +++ trunk/tests/CMakeLists.txt Thu May 28 16:48:03 2009 (r5649) @@ -269,6 +269,7 @@ BUILD_CHECK_TEST( mapping_engine engine-tests/check_mapping_engine.c ${TEST_TARGET_LIBRARIES} ) OSYNC_TESTCASE(mapping_engine mapping_engine_same_similar_conflict) +OSYNC_TESTCASE(mapping_engine mapping_engine_same_similar_conflict2) OSYNC_TESTCASE(mapping_engine mapping_engine_same_similar_conflict_multi) BUILD_CHECK_TEST( member group-tests/check_member.c ${TEST_TARGET_LIBRARIES} ) Modified: trunk/tests/engine-tests/check_mapping_engine.c ============================================================================== --- trunk/tests/engine-tests/check_mapping_engine.c Thu May 28 16:19:03 2009 (r5648) +++ trunk/tests/engine-tests/check_mapping_engine.c Thu May 28 16:48:03 2009 (r5649) @@ -96,6 +96,72 @@ } END_TEST + +/* This is similar to the previous test except the initial entries are laid + * out like: + * + * (Content) Entry A Entry B + * Member 1 xxx xxy + * Member 2: xxy + * + * This catches order-dependent issues in the mapping selection code. + */ +START_TEST (mapping_engine_same_similar_conflict2) +{ + char *testbed = setup_testbed("mapping_engine"); + char *formatdir = g_strdup_printf("%s/formats", testbed); + char *plugindir = g_strdup_printf("%s/plugins", testbed); + + + osync_testing_system_abort("cp entryA.txt entryB.txt data1/"); + osync_testing_system_abort("cp entryB.txt data2/"); + + g_setenv("MOCK_FORMAT_PATH_COMPARE_NO", "1", TRUE); + + + OSyncError *error = NULL; + OSyncGroup *group = osync_group_new(&error); + fail_unless(group != NULL, NULL); + fail_unless(error == NULL, NULL); + + osync_group_set_schemadir(group, testbed); + fail_unless(osync_group_load(group, "configs/group", &error), NULL); + fail_unless(error == NULL, NULL); + + OSyncEngine *engine = osync_engine_new(group, &error); + fail_unless(engine != NULL, NULL); + fail_unless(error == NULL, NULL); + osync_group_unref(group); + + + osync_engine_set_conflict_callback(engine, conflict_callback_fail, NULL); + + osync_engine_set_schemadir(engine, testbed); + osync_engine_set_plugindir(engine, plugindir); + osync_engine_set_formatdir(engine, formatdir); + + fail_unless(osync_engine_initialize(engine, &error), NULL); + fail_unless(error == NULL, NULL); + + fail_unless(osync_engine_synchronize_and_block(engine, &error), NULL); + fail_unless(error == NULL, NULL); + + fail_unless(osync_testing_diff("data1", "data2"), NULL); + + fail_unless(osync_engine_finalize(engine, &error), NULL); + fail_unless(error == NULL, NULL); + + osync_engine_unref(engine); + + + g_free(formatdir); + g_free(plugindir); + + destroy_testbed(testbed); +} +END_TEST + + /* Functional Test: Mapping with multiple SAME&SIMILAR entries and multiple * members * @@ -176,6 +242,7 @@ OSYNC_TESTCASE_START("mapping_engine") OSYNC_TESTCASE_ADD(mapping_engine_same_similar_conflict) +OSYNC_TESTCASE_ADD(mapping_engine_same_similar_conflict2) OSYNC_TESTCASE_ADD(mapping_engine_same_similar_conflict_multi) OSYNC_TESTCASE_END Modified: trunk/tests/mock-plugin/mock_sync.c ============================================================================== --- trunk/tests/mock-plugin/mock_sync.c Thu May 28 16:19:03 2009 (r5648) +++ trunk/tests/mock-plugin/mock_sync.c Thu May 28 16:48:03 2009 (r5649) @@ -284,6 +284,7 @@ char *path = NULL; GDir *dir = NULL; OSyncError *error = NULL; + OSyncList *sorted_dir_list = NULL; osync_trace(TRACE_ENTRY, "%s(%p, %s, %p, %p)", __func__, directory, subdir, ctx, sink); @@ -297,14 +298,24 @@ dir = g_dir_open(path, 0, &gerror); osync_assert(dir); - while ((de = g_dir_read_name(dir))) { + while((de = g_dir_read_name(dir))) { + sorted_dir_list = osync_list_insert_sorted(sorted_dir_list, + g_strdup(de), (OSyncCompareFunc)g_strcmp0); + } + + g_dir_close(dir); + + while(sorted_dir_list) { + de = sorted_dir_list->data; char *filename = g_build_filename(path, de, NULL); char *relative_filename = NULL; if (!subdir) relative_filename = g_strdup(de); else relative_filename = g_build_filename(subdir, de, NULL); - + g_free(sorted_dir_list->data); + sorted_dir_list = osync_list_remove(sorted_dir_list, sorted_dir_list->data); + osync_trace(TRACE_INTERNAL, "path2 %s %s", filename, relative_filename); if (g_file_test(filename, G_FILE_TEST_IS_REGULAR)) { @@ -368,8 +379,6 @@ } - g_dir_close(dir); - g_free(path); osync_trace(TRACE_EXIT, "%s", __func__); } |
