Fix mod testing in CI (#51843)

* Fix mod testing in CI The conversion of the tests to use GNU parallel broke the testing of JSON data for mods other than Magiclysm. Refactor the code to fix that. * Fix json errors in mods * Fix magazine selection code The fallback magazine selection code had a typo which meant that it was unable to choose any magazine type except the first listed. Fix it. Also improve the error message.
CleverRaven · Sep 27, 2021 · 73f857a · 73f857a
1 parent dbfb247
commit 73f857a
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 26 deletions.
diff --git a/build-scripts/build.sh b/build-scripts/build.sh
@@ -45,7 +45,11 @@ function run_test
 {
     set -eo pipefail
     test_exit_code=0 sed_exit_code=0 exit_code=0
-    $WINE $1 --min-duration 0.2 --use-colour yes --rng-seed time $EXTRA_TEST_OPTS $4 $5 "$2" 2>&1 | sed -E 's/^(::(warning|error|debug)[^:]*::)?/\1'"$3"'/' || test_exit_code="${PIPESTATUS[0]}" sed_exit_code="${PIPESTATUS[1]}"
+    test_bin=$1
+    prefix=$2
+    shift 2
+
+    $WINE "$test_bin" --min-duration 0.2 --use-colour yes --rng-seed time $EXTRA_TEST_OPTS "$@" 2>&1 | sed -E 's/^(::(warning|error|debug)[^:]*::)?/\1'"$prefix"'/' || test_exit_code="${PIPESTATUS[0]}" sed_exit_code="${PIPESTATUS[1]}"
     if [ "$test_exit_code" -ne "0" ]
     then
         echo "$3test exited with code $test_exit_code"
@@ -183,8 +187,8 @@ then
         make -j$num_jobs
         cd ..
         # Run regular tests
-        [ -f "${bin_path}cata_test" ] && parallel --verbose --linebuffer "run_test $(printf %q "${bin_path}")'/cata_test' {} '('{}')=> ' --user-dir=test_user_dir_{#}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
-        [ -f "${bin_path}cata_test-tiles" ] && parallel --verbose --linebuffer "run_test $(printf %q "${bin_path}")'/cata_test-tiles' {} '('{}')=> ' --user-dir=test_user_dir_{#}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
+        [ -f "${bin_path}cata_test" ] && parallel --verbose --linebuffer "run_test $(printf %q "${bin_path}")'/cata_test' '('{}')=> ' --user-dir=test_user_dir_{#} {}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
+        [ -f "${bin_path}cata_test-tiles" ] && parallel --verbose --linebuffer "run_test $(printf %q "${bin_path}")'/cata_test-tiles' '('{}')=> ' --user-dir=test_user_dir_{#} {}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
     fi
 elif [ "$NATIVE" == "android" ]
 then
@@ -205,10 +209,10 @@ else
 
     export ASAN_OPTIONS=detect_odr_violation=1
     export UBSAN_OPTIONS=print_stacktrace=1
-    parallel -j "$num_test_jobs" --verbose --linebuffer "run_test './tests/cata_test' {} '('{}')=> ' --user-dir=test_user_dir_{#}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
+    parallel -j "$num_test_jobs" --verbose --linebuffer "run_test './tests/cata_test' '('{}')=> ' --user-dir=test_user_dir_{#} {}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
     if [ -n "$MODS" ]
     then
-        parallel -j "$num_test_jobs" --verbose --linebuffer "run_test './tests/cata_test '$(printf %q "${MODS}") {} 'Mods-('{}')=> ' --user-dir=modded_{#}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
+        parallel -j "$num_test_jobs" --verbose --linebuffer "run_test './tests/cata_test' 'Mods-('{}')=> ' $(printf %q "${MODS}") --user-dir=modded_{#} {}" ::: "crafting_skill_gain" "[slow] ~crafting_skill_gain" "~[slow] ~[.]"
     fi
 
     if [ -n "$TEST_STAGE" ]
@@ -220,7 +224,7 @@ else
         ./build-scripts/get_all_mods.py | \
             while read mods
             do
-                run_test './tests/cata_test '~*' ' --user-dir=all_modded --mods='"${mods}"'
+                run_test ./tests/cata_test '(all_mods)=> ' '~*' --user-dir=all_modded --mods="${mods}"
             done
     fi
 fi

diff --git a/data/mods/Aftershock/items/inactiverobot.json b/data/mods/Aftershock/items/inactiverobot.json
@@ -437,7 +437,7 @@
     "type": "TOOL",
     "copy-from": "bot_skitterbot_base",
     "name": "inactive rat snatcher",
-    "description": "A small hexapod drone designed to hold small pests, folded into a small cube for easy of storage. Its capable of  hunting all sorts of small mammals with remarkable efficiency,  but shouldn't be expected to handle anything much bigger than a rat.",
+    "description": "A small hexapod drone designed to hold small pests, folded into a small cube for easy of storage.  Its capable of hunting all sorts of small mammals with remarkable efficiency, but shouldn't be expected to handle anything much bigger than a rat.",
     "color": "dark_gray",
     "weight": "750 g",
     "volume": "1 L",

diff --git a/data/mods/Aftershock/recipes/recipe_overrides.json b/data/mods/Aftershock/recipes/recipe_overrides.json
@@ -131,8 +131,8 @@
     "activity_level": "LIGHT_EXERCISE",
     "result": "foie_gras",
     "category": "CC_FOOD",
-    "skill_used": "cooking",
     "subcategory": "CSC_FOOD_MEAT",
+    "skill_used": "cooking",
     "difficulty": 6,
     "charges": 1,
     "time": 20000,

diff --git a/data/mods/CrazyCataclysm/crazy_items.json b/data/mods/CrazyCataclysm/crazy_items.json
@@ -120,7 +120,7 @@
     "color": "light_gray",
     "ammo": [ "gasoline" ],
     "charges_per_use": 1,
-    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 } } ],
+    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 }, "watertight": true } ],
     "techniques": [ "RAPID", "WBLOCK_2" ],
     "use_action": {
       "type": "fireweapon_off",
@@ -148,7 +148,7 @@
     "symbol": "/",
     "color": "red",
     "ammo": [ "gasoline" ],
-    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 } } ],
+    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 }, "watertight": true } ],
     "turns_per_charge": 30,
     "revert_to": "firekatana_off",
     "techniques": [ "RAPID", "WBLOCK_2" ],
@@ -182,7 +182,7 @@
     "color": "light_gray",
     "ammo": [ "gasoline" ],
     "charges_per_use": 1,
-    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 } } ],
+    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 }, "watertight": true } ],
     "techniques": [ "WBLOCK_1", "WIDE", "BRUTAL", "SWEEP" ],
     "use_action": {
       "type": "fireweapon_off",
@@ -211,7 +211,7 @@
     "symbol": "/",
     "color": "red",
     "ammo": [ "gasoline" ],
-    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 } } ],
+    "pocket_data": [ { "pocket_type": "MAGAZINE", "rigid": true, "ammo_restriction": { "gasoline": 50 }, "watertight": true } ],
     "turns_per_charge": 30,
     "revert_to": "zweifire_off",
     "techniques": [ "WBLOCK_1", "WIDE", "BRUTAL", "SWEEP" ],

diff --git a/data/mods/Dark-Skies-Above/blacklists/item_blacklist.json b/data/mods/Dark-Skies-Above/blacklists/item_blacklist.json
@@ -109,7 +109,6 @@
       "bio_remote",
       "bio_resonator",
       "bio_scent_mask",
-      "bio_scent_vision",
       "bio_shock",
       "bio_shockwave",
       "bio_shotgun",
@@ -134,7 +133,6 @@
       "bot_rifleturret",
       "bot_crows_m240",
       "bot_turret_riot",
-      "cerberus_laser",
       "coilgun",
       "depowered_armor",
       "depowered_helmet",
@@ -165,13 +163,10 @@
       "iv_mutagen_troglobite",
       "iv_mutagen_ursine",
       "iv_purifier",
-      "l_bak_223",
-      "laser_pack",
       "laser_rifle",
       "leg",
       "light_atomic_battery_cell",
       "light_minus_atomic_battery_cell",
-      "lwfeed",
       "marloss_berry",
       "marloss_gel",
       "marloss_scarf",
@@ -214,7 +209,6 @@
       "purifier",
       "razorclaw_roe",
       "rebar_rail",
-      "rebar_rifle",
       "recipe_caseless",
       "reloaded_5x50dart",
       "resin_chunk",
@@ -233,10 +227,7 @@
       "rm802",
       "rm88_battle_rifle",
       "steel_rail",
-      "taint_tornado",
-      "unbio_blaster_gun",
       "v29",
-      "v29_cheap",
       "wine_mycus"
     ]
   }

diff --git a/data/mods/National_Guard_Camp/military.json b/data/mods/National_Guard_Camp/military.json
@@ -85,13 +85,13 @@
     "type": "MONSTER",
     "name": "W11H10",
     "description": "Wraitheon (11H) Anti-Tank Infantry 10 Level.  Part of Wraitheon's series of one-to-one auxiliaries designed to seamlessly integrate with more traditional forces.",
-    "starting_ammo": { "40mm_incendiary": 8 },
+    "starting_ammo": { "40x46mm_m433": 8 },
     "special_attacks": [
       {
         "type": "gun",
         "cooldown": 3,
         "gun_type": "m320",
-        "ammo_type": "40mm_incendiary",
+        "ammo_type": "40x46mm_m433",
         "fake_skills": [ [ "gun", 1 ], [ "launcher", 1 ] ],
         "fake_dex": 8,
         "ranges": [ [ 5, 20, "DEFAULT" ] ]

diff --git a/src/item.cpp b/src/item.cpp
@@ -627,13 +627,23 @@ item &item::ammo_set( const itype_id &ammo, int qty )
             if( mag_item.ammo_capacity( ammo_type ) < qty ) {
                 std::vector<item> opts;
                 for( const itype_id &mag_type : mags ) {
-                    if( mag->magazine->type.count( ammo_type ) ) {
+                    if( mag_type->magazine->type.count( ammo_type ) ) {
                         opts.emplace_back( mag_type );
                     }
                 }
                 if( opts.empty() ) {
-                    debugmsg( "Cannot find magazine with enough %s ammo capacity for %s", ammo.c_str(),
-                              typeId().c_str() );
+                    const std::string magazines_str = enumerate_as_string( mags,
+                    []( const itype_id & mag ) {
+                        return string_format(
+                                   "%s (taking %s)", mag.str(),
+                                   enumerate_as_string( mag->magazine->type,
+                        []( const ammotype & a ) {
+                            return a.str();
+                        } ) );
+                    } );
+                    debugmsg( "Cannot find magazine fitting %s with any capacity for ammo %s "
+                              "(ammotype %s).  Magazines considered were %s",
+                              typeId().str(), ammo.str(), ammo_type.str(), magazines_str );
                     return *this;
                 }
                 std::sort( opts.begin(), opts.end(), [&ammo_type]( const item & lhs, const item & rhs ) {