Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a test for missing texture files #321

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

klingbolt
Copy link
Contributor

@klingbolt klingbolt commented Jul 2, 2024

This test does not have files to support the texture paths.

Double errors are expected as there is a check when writing the proto and a check when writing the xml

@klingbolt klingbolt changed the title Added a test for having a large array of textures. This one does not … Added a test for having a large array of textures Jul 2, 2024
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full Diff
--- xml_converter/integration_tests/test_cases/large_texture_array_invalid/output_proto/markers.bin.textproto._old	2024-07-02 00:33:42.423665756 +0000
+++ xml_converter/integration_tests/test_cases/large_texture_array_invalid/output_proto/markers.bin.textproto._new	2024-07-02 00:33:42.431665838 +0000
@@ -0,0 +1,703 @@
+category {
+  name: "MarkerPack"
+  children {
+    name: "My Category"
+    icon {
+      texture_id: 1
+      map_id: 50
+    }
+    icon {
+      texture_id: 2
+      map_id: 50
+    }
+    icon {
+      texture_id: 3
+      map_id: 50
+    }
+    icon {
+      texture_id: 4
+      map_id: 50
+    }
+    icon {
+      texture_id: 5
+      map_id: 50
+    }
+    icon {
+      texture_id: 6
+      map_id: 50
+    }
+    icon {
+      texture_id: 7
+      map_id: 50
+    }
+    icon {
+      texture_id: 8
+      map_id: 50
+    }
+    icon {
+      texture_id: 9
+      map_id: 50
+    }
+    icon {
+      texture_id: 10
+      map_id: 50
+    }
+    icon {
+      texture_id: 11
+      map_id: 50
+    }
+    icon {
+      texture_id: 12
+      map_id: 50
+    }
+    icon {
+      texture_id: 13
+      map_id: 50
+    }
+    icon {
+      texture_id: 14
+      map_id: 50
+    }
+    icon {
+      texture_id: 15
+      map_id: 50
+    }
+    icon {
+      texture_id: 16
+      map_id: 50
+    }
+    icon {
+      texture_id: 17
+      map_id: 50
+    }
+    icon {
+      texture_id: 18
+      map_id: 50
+    }
+    icon {
+      texture_id: 19
+      map_id: 50
+    }
+    icon {
+      texture_id: 20
+      map_id: 50
+    }
+    icon {
+      texture_id: 21
+      map_id: 50
+    }
+    icon {
+      texture_id: 22
+      map_id: 50
+    }
+    icon {
+      texture_id: 23
+      map_id: 50
+    }
+    icon {
+      texture_id: 24
+      map_id: 50
+    }
+    icon {
+      texture_id: 25
+      map_id: 50
+    }
+    icon {
+      texture_id: 26
+      map_id: 50
+    }
+    icon {
+      texture_id: 27
+      map_id: 50
+    }
+    icon {
+      texture_id: 28
+      map_id: 50
+    }
+    icon {
+      texture_id: 29
+      map_id: 50
+    }
+    icon {
+      texture_id: 30
+      map_id: 50
+    }
+    icon {
+      texture_id: 31
+      map_id: 50
+    }
+    icon {
+      texture_id: 32
+      map_id: 50
+    }
+    icon {
+      texture_id: 33
+      map_id: 50
+    }
+    icon {
+      texture_id: 34
+      map_id: 50
+    }
+    icon {
+      texture_id: 35
+      map_id: 50
+    }
+    icon {
+      texture_id: 36
+      map_id: 50
+    }
+    icon {
+      texture_id: 37
+      map_id: 50
+    }
+    icon {
+      texture_id: 38
+      map_id: 50
+    }
+    icon {
+      texture_id: 39
+      map_id: 50
+    }
+    icon {
+      texture_id: 40
+      map_id: 50
+    }
+    icon {
+      texture_id: 41
+      map_id: 50
+    }
+    icon {
+      texture_id: 42
+      map_id: 50
+    }
+    icon {
+      texture_id: 43
+      map_id: 50
+    }
+    icon {
+      texture_id: 44
+      map_id: 50
+    }
+    icon {
+      texture_id: 45
+      map_id: 50
+    }
+    icon {
+      texture_id: 46
+      map_id: 50
+    }
+    icon {
+      texture_id: 47
+      map_id: 50
+    }
+    icon {
+      texture_id: 48
+      map_id: 50
+    }
+    icon {
+      texture_id: 49
+      map_id: 50
+    }
+    icon {
+      texture_id: 50
+      map_id: 50
+    }
+    icon {
+      texture_id: 51
+      map_id: 50
+    }
+    icon {
+      texture_id: 52
+      map_id: 50
+    }
+    icon {
+      texture_id: 53
+      map_id: 50
+    }
+    icon {
+      texture_id: 54
+      map_id: 50
+    }
+    icon {
+      texture_id: 55
+      map_id: 50
+    }
+    icon {
+      texture_id: 56
+      map_id: 50
+    }
+    icon {
+      texture_id: 57
+      map_id: 50
+    }
+    icon {
+      texture_id: 58
+      map_id: 50
+    }
+    icon {
+      texture_id: 59
+      map_id: 50
+    }
+    icon {
+      texture_id: 60
+      map_id: 50
+    }
+    icon {
+      texture_id: 61
+      map_id: 50
+    }
+    icon {
+      texture_id: 62
+      map_id: 50
+    }
+    icon {
+      texture_id: 63
+      map_id: 50
+    }
+    icon {
+      texture_id: 64
+      map_id: 50
+    }
+    icon {
+      texture_id: 65
+      map_id: 50
+    }
+    icon {
+      texture_id: 66
+      map_id: 50
+    }
+    icon {
+      texture_id: 67
+      map_id: 50
+    }
+    icon {
+      texture_id: 68
+      map_id: 50
+    }
+    icon {
+      texture_id: 69
+      map_id: 50
+    }
+    icon {
+      texture_id: 70
+      map_id: 50
+    }
+    icon {
+      texture_id: 71
+      map_id: 50
+    }
+    icon {
+      texture_id: 72
+      map_id: 50
+    }
+    icon {
+      texture_id: 73
+      map_id: 50
+    }
+    icon {
+      texture_id: 74
+      map_id: 50
+    }
+    icon {
+      texture_id: 75
+      map_id: 50
+    }
+    icon {
+      texture_id: 76
+      map_id: 50
+    }
+    icon {
+      texture_id: 77
+      map_id: 50
+    }
+    icon {
+      texture_id: 78
+      map_id: 50
+    }
+    icon {
+      texture_id: 79
+      map_id: 50
+    }
+    icon {
+      texture_id: 80
+      map_id: 50
+    }
+    icon {
+      texture_id: 81
+      map_id: 50
+    }
+    icon {
+      texture_id: 82
+      map_id: 50
+    }
+    icon {
+      texture_id: 83
+      map_id: 50
+    }
+    icon {
+      texture_id: 84
+      map_id: 50
+    }
+    icon {
+      texture_id: 85
+      map_id: 50
+    }
+    icon {
+      texture_id: 86
+      map_id: 50
+    }
+    icon {
+      texture_id: 87
+      map_id: 50
+    }
+    icon {
+      texture_id: 88
+      map_id: 50
+    }
+    icon {
+      texture_id: 89
+      map_id: 50
+    }
+    icon {
+      texture_id: 90
+      map_id: 50
+    }
+    icon {
+      texture_id: 91
+      map_id: 50
+    }
+    icon {
+      texture_id: 92
+      map_id: 50
+    }
+    icon {
+      texture_id: 93
+      map_id: 50
+    }
+    icon {
+      texture_id: 94
+      map_id: 50
+    }
+    icon {
+      texture_id: 95
+      map_id: 50
+    }
+    icon {
+      texture_id: 96
+      map_id: 50
+    }
+    icon {
+      texture_id: 97
+      map_id: 50
+    }
+    icon {
+      texture_id: 98
+      map_id: 50
+    }
+    icon {
+      texture_id: 99
+      map_id: 50
+    }
+    id: "\370\030\326\316\303\200\251!"
+  }
+  id: "(\350\314\006_cHb"
+}
+textures {
+}
+textures {
+  filepath: "texture_01.png"
+}
+textures {
+  filepath: "texture_02.png"
+}
+textures {
+  filepath: "texture_03.png"
+}
+textures {
+  filepath: "texture_04.png"
+}
+textures {
+  filepath: "texture_05.png"
+}
+textures {
+  filepath: "texture_06.png"
+}
+textures {
+  filepath: "texture_07.png"
+}
+textures {
+  filepath: "texture_08.png"
+}
+textures {
+  filepath: "texture_09.png"
+}
+textures {
+  filepath: "texture_10.png"
+}
+textures {
+  filepath: "texture_11.png"
+}
+textures {
+  filepath: "texture_12.png"
+}
+textures {
+  filepath: "texture_13.png"
+}
+textures {
+  filepath: "texture_14.png"
+}
+textures {
+  filepath: "texture_15.png"
+}
+textures {
+  filepath: "texture_16.png"
+}
+textures {
+  filepath: "texture_17.png"
+}
+textures {
+  filepath: "texture_18.png"
+}
+textures {
+  filepath: "texture_19.png"
+}
+textures {
+  filepath: "texture_20.png"
+}
+textures {
+  filepath: "texture_21.png"
+}
+textures {
+  filepath: "texture_22.png"
+}
+textures {
+  filepath: "texture_23.png"
+}
+textures {
+  filepath: "texture_24.png"
+}
+textures {
+  filepath: "texture_25.png"
+}
+textures {
+  filepath: "texture_26.png"
+}
+textures {
+  filepath: "texture_27.png"
+}
+textures {
+  filepath: "texture_28.png"
+}
+textures {
+  filepath: "texture_29.png"
+}
+textures {
+  filepath: "texture_30.png"
+}
+textures {
+  filepath: "texture_31.png"
+}
+textures {
+  filepath: "texture_32.png"
+}
+textures {
+  filepath: "texture_33.png"
+}
+textures {
+  filepath: "texture_34.png"
+}
+textures {
+  filepath: "texture_35.png"
+}
+textures {
+  filepath: "texture_36.png"
+}
+textures {
+  filepath: "texture_37.png"
+}
+textures {
+  filepath: "texture_38.png"
+}
+textures {
+  filepath: "texture_39.png"
+}
+textures {
+  filepath: "texture_40.png"
+}
+textures {
+  filepath: "texture_41.png"
+}
+textures {
+  filepath: "texture_42.png"
+}
+textures {
+  filepath: "texture_43.png"
+}
+textures {
+  filepath: "texture_44.png"
+}
+textures {
+  filepath: "texture_45.png"
+}
+textures {
+  filepath: "texture_46.png"
+}
+textures {
+  filepath: "texture_47.png"
+}
+textures {
+  filepath: "texture_48.png"
+}
+textures {
+  filepath: "texture_49.png"
+}
+textures {
+  filepath: "texture_50.png"
+}
+textures {
+  filepath: "texture_51.png"
+}
+textures {
+  filepath: "texture_52.png"
+}
+textures {
+  filepath: "texture_53.png"
+}
+textures {
+  filepath: "texture_54.png"
+}
+textures {
+  filepath: "texture_55.png"
+}
+textures {
+  filepath: "texture_56.png"
+}
+textures {
+  filepath: "texture_57.png"
+}
+textures {
+  filepath: "texture_58.png"
+}
+textures {
+  filepath: "texture_59.png"
+}
+textures {
+  filepath: "texture_60.png"
+}
+textures {
+  filepath: "texture_61.png"
+}
+textures {
+  filepath: "texture_62.png"
+}
+textures {
+  filepath: "texture_63.png"
+}
+textures {
+  filepath: "texture_64.png"
+}
+textures {
+  filepath: "texture_65.png"
+}
+textures {
+  filepath: "texture_66.png"
+}
+textures {
+  filepath: "texture_67.png"
+}
+textures {
+  filepath: "texture_68.png"
+}
+textures {
+  filepath: "texture_69.png"
+}
+textures {
+  filepath: "texture_70.png"
+}
+textures {
+  filepath: "texture_71.png"
+}
+textures {
+  filepath: "texture_72.png"
+}
+textures {
+  filepath: "texture_73.png"
+}
+textures {
+  filepath: "texture_74.png"
+}
+textures {
+  filepath: "texture_75.png"
+}
+textures {
+  filepath: "texture_76.png"
+}
+textures {
+  filepath: "texture_77.png"
+}
+textures {
+  filepath: "texture_78.png"
+}
+textures {
+  filepath: "texture_79.png"
+}
+textures {
+  filepath: "texture_80.png"
+}
+textures {
+  filepath: "texture_81.png"
+}
+textures {
+  filepath: "texture_82.png"
+}
+textures {
+  filepath: "texture_83.png"
+}
+textures {
+  filepath: "texture_84.png"
+}
+textures {
+  filepath: "texture_85.png"
+}
+textures {
+  filepath: "texture_86.png"
+}
+textures {
+  filepath: "texture_87.png"
+}
+textures {
+  filepath: "texture_88.png"
+}
+textures {
+  filepath: "texture_89.png"
+}
+textures {
+  filepath: "texture_90.png"
+}
+textures {
+  filepath: "texture_91.png"
+}
+textures {
+  filepath: "texture_92.png"
+}
+textures {
+  filepath: "texture_93.png"
+}
+textures {
+  filepath: "texture_94.png"
+}
+textures {
+  filepath: "texture_95.png"
+}
+textures {
+  filepath: "texture_96.png"
+}
+textures {
+  filepath: "texture_97.png"
+}
+textures {
+  filepath: "texture_98.png"
+}
+textures {
+  filepath: "texture_99.png"
+}

@AsherGlick
Copy link
Owner

What is the purpose of this test? I dont see any associated bug or fix related to it.

@klingbolt
Copy link
Contributor Author

What is the purpose of this test? I dont see any associated bug or fix related to it.

The bug I was trying to was eventually solved by #320
I had two purposes for this.

  • I wanted a file with a large number of textures so that i could evaluate performance while working on the Changed Textures to be assigned by waypoint texture ID #319.
  • I wanted to ensure the expected behavior which is that the path in the texture field that does not refer to a real file is still passed in and doesn't leave an empty texture array.

@AsherGlick
Copy link
Owner

Unit tests or integration tests are not generally a good place to put performance tests. Performance tests are by definition slow and will just end up slowing down the unit tests.

Ensuring that the expected behavior occurs as described can be done with a smaller better named test then the one written in this PR. Effectively this test is just checking the value of IconFile 100 times. Which would be the same test if it was done a single time instead.

@klingbolt
Copy link
Contributor Author

Makes sense. I'll change this to a smaller test then

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full Diff
--- xml_converter/integration_tests/test_cases/texture_invalid/output_proto/markers.bin.textproto._old	2024-07-02 02:56:47.850164729 +0000
+++ xml_converter/integration_tests/test_cases/texture_invalid/output_proto/markers.bin.textproto._new	2024-07-02 02:56:47.858164695 +0000
@@ -0,0 +1,13 @@
+category {
+  name: "MyCategory"
+  icon {
+    texture_id: 1
+    map_id: 50
+  }
+  id: "(\350\314\006\302\223^\226"
+}
+textures {
+}
+textures {
+  filepath: "texture_01.png"
+}

@klingbolt klingbolt changed the title Added a test for having a large array of textures Added a test for missing texture files Jul 2, 2024
@AsherGlick AsherGlick merged commit 3ef128d into AsherGlick:xml_converter Jul 2, 2024
7 checks passed
@klingbolt klingbolt deleted the texture_invalid branch September 11, 2024 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants