Skip to content

Commit a08196a

Browse files
CharliePooleCharliePoole
committed
Changes from code review
1 parent 671710c commit a08196a

File tree

4 files changed

+28
-70
lines changed

4 files changed

+28
-70
lines changed

‎src/NUnitEngine/nunit.engine.api/TestPackage.cs‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,11 @@ public XmlSchema GetSchema()
150150
}
151151

152152
/// <inheritdoc />
153-
public void ReadXml(XmlReader xmlReader) => ReadPackageFromXml(this, xmlReader);
154-
155-
// Read a single package and its subpackages from the xml reader
156-
private void ReadPackageFromXml(TestPackage package, XmlReader xmlReader)
153+
public void ReadXml(XmlReader xmlReader)
157154
{
158-
package.ID = xmlReader.GetAttribute("id");
159-
package.FullName = xmlReader.GetAttribute("fullname");
155+
var currentPackage = this;
156+
currentPackage.ID = xmlReader.GetAttribute("id");
157+
currentPackage.FullName = xmlReader.GetAttribute("fullname");
160158
if (!xmlReader.IsEmptyElement)
161159
{
162160
while (xmlReader.Read())
@@ -167,16 +165,17 @@ private void ReadPackageFromXml(TestPackage package, XmlReader xmlReader)
167165
switch(xmlReader.Name)
168166
{
169167
case "Settings":
168+
// We don't use AddSettings, which copies settings downward.
169+
// Instead, each package handles it's own settings.
170170
while (xmlReader.MoveToNextAttribute())
171-
package.AddSetting(xmlReader.Name, xmlReader.Value);
172-
171+
currentPackage.Settings.Add(xmlReader.Name, xmlReader.Value);
173172
xmlReader.MoveToElement();
174173
break;
175174

176175
case "TestPackage":
177-
TestPackage testPackage = new TestPackage();
178-
ReadPackageFromXml(testPackage, xmlReader);
179-
package.SubPackages.Add(testPackage);
176+
TestPackage subPackage = new TestPackage();
177+
subPackage.ReadXml(xmlReader);
178+
currentPackage.SubPackages.Add(subPackage);
180179
break;
181180
}
182181
break;
@@ -191,37 +190,34 @@ private void ReadPackageFromXml(TestPackage package, XmlReader xmlReader)
191190
}
192191
}
193192

194-
package.SubPackages.Add(package);
193+
throw new Exception("Invalid XML: TestPackage Element not terminated.");
195194
}
196195
}
197196

198197
/// <inheritdoc />
199-
public void WriteXml(XmlWriter xmlWriter) => WritePackageToXml(this, xmlWriter);
200-
201-
// Write a single package and its subpackages to the xmlWriter
202-
private void WritePackageToXml(TestPackage package, XmlWriter xmlWriter)
198+
public void WriteXml(XmlWriter xmlWriter)
203199
{
204200
// Write ID and FullName
205-
xmlWriter.WriteAttributeString("id", package.ID);
206-
if (package.FullName != null)
207-
xmlWriter.WriteAttributeString("fullname", package.FullName);
201+
xmlWriter.WriteAttributeString("id", ID);
202+
if (FullName != null)
203+
xmlWriter.WriteAttributeString("fullname", FullName);
208204

209205
// Write Settings
210-
if (package.Settings.Count != 0)
206+
if (Settings.Count != 0)
211207
{
212208
xmlWriter.WriteStartElement("Settings");
213209

214-
foreach (KeyValuePair<string, object> setting in package.Settings)
210+
foreach (KeyValuePair<string, object> setting in Settings)
215211
xmlWriter.WriteAttributeString(setting.Key, setting.Value.ToString());
216212

217213
xmlWriter.WriteEndElement();
218214
}
219215

220216
// Write any SubPackages recursively
221-
foreach (TestPackage subPackage in package.SubPackages)
217+
foreach (TestPackage subPackage in SubPackages)
222218
{
223219
xmlWriter.WriteStartElement("TestPackage");
224-
WritePackageToXml(subPackage, xmlWriter);
220+
subPackage.WriteXml(xmlWriter);
225221
xmlWriter.WriteEndElement();
226222
}
227223
}

‎src/NUnitEngine/nunit.engine.core/Communication/Messages/TestEngineMessage.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@
55
namespace NUnit.Engine.Communication.Messages
66
{
77
[Serializable]
8-
public class TestEngineMessage
8+
public sealed class TestEngineMessage
99
{
1010
public TestEngineMessage(string code, string data)
1111
{
1212
Code = code;
1313
Data = data;
1414
}
1515

16-
protected TestEngineMessage() { }
17-
1816
public string Code { get; }
1917
public string Data { get; }
2018
}

‎src/NUnitEngine/nunit.engine.core/Communication/Protocols/BinarySerializationProtocol.cs‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -99,44 +99,6 @@ public void Reset()
9999
}
100100
}
101101

102-
///// <summary>
103-
///// Serializes a message to a byte array to send to remote application.
104-
///// </summary>
105-
///// <param name="message">Message to be serialized</param>
106-
///// <returns>
107-
///// A byte[] containing the message itself, without a prefixed
108-
///// length, serialized according to the protocol.
109-
///// </returns>
110-
//internal byte[] SerializeMessage(object message)
111-
//{
112-
// using (var memoryStream = new MemoryStream())
113-
// {
114-
// new BinaryFormatter().Serialize(memoryStream, message);
115-
// return memoryStream.ToArray();
116-
// }
117-
//}
118-
119-
///// <summary>
120-
///// Deserializes a message contained in a byte array.
121-
///// </summary>
122-
///// <param name="bytes">A byte[] containing just the message, without a length prefix</param>
123-
///// <returns>An object representing the message encoded in the byte array</returns>
124-
//internal object DeserializeMessage(byte[] bytes)
125-
//{
126-
// using (var memoryStream = new MemoryStream(bytes))
127-
// {
128-
// try
129-
// {
130-
// return new BinaryFormatter().Deserialize(memoryStream);
131-
// }
132-
// catch (Exception exception)
133-
// {
134-
// Reset(); // reset the received memory stream before the exception is rethrown - otherwise the same erroneous message is received again and again
135-
// throw new SerializationException("error while deserializing message", exception);
136-
// }
137-
// }
138-
//}
139-
140102
/// <summary>
141103
/// This method tries to read a single message and add to the messages collection.
142104
/// </summary>

‎src/NUnitEngine/nunit.engine.core/Internal/TestPackageExtensions.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,14 @@ private static void AccumulatePackages(TestPackage package, IList<TestPackage> s
4646
public static string ToXml(this TestPackage package)
4747
{
4848
var writer = new StringWriter();
49-
var xmlWriter = XmlWriter.Create(writer, new XmlWriterSettings() { OmitXmlDeclaration = true });
50-
var serializer = new XmlSerializer(typeof(TestPackage));
51-
serializer.Serialize(xmlWriter, package);
52-
xmlWriter.Flush();
53-
xmlWriter.Close();
5449

50+
using (var xmlWriter = XmlWriter.Create(writer, new XmlWriterSettings() { OmitXmlDeclaration = true }))
51+
{
52+
xmlWriter.WriteStartElement(nameof(TestPackage));
53+
package.WriteXml(xmlWriter);
54+
xmlWriter.WriteEndElement();
55+
}
56+
5557
return writer.ToString();
5658
}
5759

@@ -80,7 +82,7 @@ bool ReadTestPackageElement()
8082
{
8183
while (xmlReader.Read())
8284
if (xmlReader.NodeType == XmlNodeType.Element)
83-
return xmlReader.Name == "TestPackage";
85+
return xmlReader.Name == nameof(TestPackage);
8486
return false;
8587
}
8688
}

0 commit comments

Comments
 (0)