Skip to content

Commit

Permalink
Throw instead of returning null in XNodeReader.GetAttribute(int) (#44589
Browse files Browse the repository at this point in the history 
)

* Throw exception on non-interactive mode and invalid index instead of returning null

* Adapt unit tests to exceptions

* Move tests from XCMLReaderAttrTest to XNodeReaderAttributeTests.

Remove these test cases from FunctionTests' children

* Migrate attribute tests toward XUnit

* Remove migrated tests and distinguish between int and string indexer

* Throw ArgumentOutOfRangeException when index is above upper bound and update tests
  • Loading branch information
@lennartb-
lennartb- authored Feb 16, 2021
1 parent 6047711 commit 54f6250
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 123 deletions.
11 changes: 5 additions & 6 deletions src/libraries/System.Private.Xml.Linq/src/System/Xml/Linq/XNodeReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,17 +554,16 @@ public override void Close()

public override string GetAttribute(int index)
{
// https://github.com/dotnet/runtime/issues/44287
// We should replace returning null with ArgumentOutOfRangeException
// In case of not interactive state likely we should throw InvalidOperationException
if (!IsInteractive)
{
return null!;
throw new InvalidOperationException(SR.InvalidOperation_ExpectedInteractive);
}

if (index < 0)
{
return null!;
throw new ArgumentOutOfRangeException(nameof(index));
}

XElement? e = GetElementInAttributeScope();
if (e != null)
{
Expand All @@ -584,7 +583,7 @@ public override string GetAttribute(int index)
} while (a != e.lastAttr);
}
}
return null!;
throw new ArgumentOutOfRangeException(nameof(index));
}

public override string? LookupNamespace(string prefix)
Expand Down
65 changes: 0 additions & 65 deletions src/libraries/System.Private.Xml.Linq/tests/xNodeReader/CXMLReaderAttrTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.IO;
using System.Xml;
using Microsoft.Test.ModuleCore;

Expand Down Expand Up @@ -526,38 +525,6 @@ public void GetAttributeWithMoveAttrSingleQ()
TestLog.Compare(str, DataReader.Value, "Ordinal (" + i + "): Compare MoveToAttribute[i] and string");
}
}

//[Variation("GetAttribute(i) NegativeOneOrdinal", Priority = 0)]
public void NegativeOneOrdinal()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader.GetAttribute(-1);
}

//[Variation("GetAttribute(i) FieldCountOrdinal")]
public void FieldCountOrdinal()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT0");
string str = DataReader.GetAttribute(DataReader.AttributeCount);
}

//[Variation("GetAttribute(i) OrdinalPlusOne", Priority = 0)]
public void OrdinalPlusOne()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader.GetAttribute(DataReader.AttributeCount + 1);
}

//[Variation("GetAttribute(i) OrdinalMinusOne")]
public void OrdinalMinusOne()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader.GetAttribute(-2);
}
}

//[TestCase(Name = "GetAttributeName", Desc = "GetAttributeName")]
Expand Down Expand Up @@ -810,38 +777,6 @@ public void OrdinalWithMoveAttrSingleQ()
TestLog.Compare(str, DataReader.Value, "Ordinal (" + i + "): Compare MoveToAttribute[i] and string");
}
}

//[Variation("ThisOrdinal NegativeOneOrdinal", Priority = 0)]
public void NegativeOneOrdinal()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader[-1];
}

//[Variation("ThisOrdinal FieldCountOrdinal")]
public void FieldCountOrdinal()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT0");
string str = DataReader[DataReader.AttributeCount];
}

//[Variation("ThisOrdinal OrdinalPlusOne", Priority = 0)]
public void OrdinalPlusOne()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader[DataReader.AttributeCount + 1];
}

//[Variation("ThisOrdinal OrdinalMinusOne")]
public void OrdinalMinusOne()
{
XmlReader DataReader = GetReader();
PositionOnElement(DataReader, "ACT1");
string str = DataReader[-2];
}
}

//[TestCase(Name = "MoveToAttributeOrdinal", Desc = "MoveToAttributeOrdinal")]
Expand Down
40 changes: 1 addition & 39 deletions src/libraries/System.Private.Xml.Linq/tests/xNodeReader/ErrorConditions.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Test.ModuleCore;
using System;
using System.Collections.Generic;
using System.Xml;
using System.Xml.Linq;
using Microsoft.Test.ModuleCore;

namespace CoreXml.Test.XLinq
{
Expand All @@ -15,44 +15,6 @@ public partial class XNodeReaderTests : XLinqTestCase
{
public partial class ErrorConditions : BridgeHelpers
{
//[Variation(Desc = "Move to Attribute using []")]
public void Variation1()
{
foreach (XNode n in GetXNodeR())
{
using (XmlReader r = n.CreateReader())
{
TestLog.Compare(r[-100000], null, "Error");
TestLog.Compare(r[-1], null, "Error");
TestLog.Compare(r[0], null, "Error");
TestLog.Compare(r[100000], null, "Error");
TestLog.Compare(r[null], null, "Error");
TestLog.Compare(r[null, null], null, "Error");
TestLog.Compare(r[""], null, "Error");
TestLog.Compare(r["", ""], null, "Error");
}
}
}

//[Variation(Desc = "GetAttribute")]
public void Variation2()
{
foreach (XNode n in GetXNodeR())
{
using (XmlReader r = n.CreateReader())
{
TestLog.Compare(r.GetAttribute(-100000), null, "Error");
TestLog.Compare(r.GetAttribute(-1), null, "Error");
TestLog.Compare(r.GetAttribute(0), null, "Error");
TestLog.Compare(r.GetAttribute(100000), null, "Error");
TestLog.Compare(r.GetAttribute(null), null, "Error");
TestLog.Compare(r.GetAttribute(null, null), null, "Error");
TestLog.Compare(r.GetAttribute(""), null, "Error");
TestLog.Compare(r.GetAttribute("", ""), null, "Error");
}
}
}

//[Variation(Desc = "IsStartElement")]
public void Variation3()
{
Expand Down
10 changes: 0 additions & 10 deletions src/libraries/System.Private.Xml.Linq/tests/xNodeReader/FunctionalTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,6 @@ public override void AddChildren()
this.AddChild(new TestVariation(OrdinalWithGetAttrSingleQ) { Attribute = new VariationAttribute("GetAttribute[i] Verify with This[i] - Single Quote") });
this.AddChild(new TestVariation(GetAttributeWithMoveAttrDoubleQ) { Attribute = new VariationAttribute("GetAttribute(i) Verify with MoveToAttribute[i] - Double Quote") { Priority = 0 } });
this.AddChild(new TestVariation(GetAttributeWithMoveAttrSingleQ) { Attribute = new VariationAttribute("GetAttribute(i) Verify with MoveToAttribute[i] - Single Quote") });
this.AddChild(new TestVariation(NegativeOneOrdinal) { Attribute = new VariationAttribute("GetAttribute(i) NegativeOneOrdinal") { Priority = 0 } });
this.AddChild(new TestVariation(FieldCountOrdinal) { Attribute = new VariationAttribute("GetAttribute(i) FieldCountOrdinal") });
this.AddChild(new TestVariation(OrdinalPlusOne) { Attribute = new VariationAttribute("GetAttribute(i) OrdinalPlusOne") { Priority = 0 } });
this.AddChild(new TestVariation(OrdinalMinusOne) { Attribute = new VariationAttribute("GetAttribute(i) OrdinalMinusOne") });
}
}
public partial class TCGetAttributeName : BridgeHelpers
Expand Down Expand Up @@ -344,10 +340,6 @@ public override void AddChildren()
this.AddChild(new TestVariation(OrdinalWithGetAttrSingleQ) { Attribute = new VariationAttribute("This[i] Verify with GetAttribute[i] - Single Quote") });
this.AddChild(new TestVariation(OrdinalWithMoveAttrSingleQ) { Attribute = new VariationAttribute("This[i] Verify with MoveToAttribute[i] - Single Quote") });
this.AddChild(new TestVariation(OrdinalWithMoveAttrDoubleQ) { Attribute = new VariationAttribute("This[i] Verify with MoveToAttribute[i] - Double Quote") { Priority = 0 } });
this.AddChild(new TestVariation(NegativeOneOrdinal) { Attribute = new VariationAttribute("ThisOrdinal NegativeOneOrdinal") { Priority = 0 } });
this.AddChild(new TestVariation(FieldCountOrdinal) { Attribute = new VariationAttribute("ThisOrdinal FieldCountOrdinal") });
this.AddChild(new TestVariation(OrdinalPlusOne) { Attribute = new VariationAttribute("ThisOrdinal OrdinalPlusOne") { Priority = 0 } });
this.AddChild(new TestVariation(OrdinalMinusOne) { Attribute = new VariationAttribute("ThisOrdinal OrdinalMinusOne") });
}
}
public partial class TCMoveToAttributeOrdinal : BridgeHelpers
Expand Down Expand Up @@ -580,8 +572,6 @@ public partial class ErrorConditions : BridgeHelpers
// Test Case
public override void AddChildren()
{
this.AddChild(new TestVariation(Variation1) { Attribute = new VariationAttribute("Move to Attribute using []") });
this.AddChild(new TestVariation(Variation2) { Attribute = new VariationAttribute("GetAttribute") });
this.AddChild(new TestVariation(Variation3) { Attribute = new VariationAttribute("IsStartElement") });
this.AddChild(new TestVariation(Variation4) { Attribute = new VariationAttribute("LookupNamespace") });
this.AddChild(new TestVariation(Variation5) { Attribute = new VariationAttribute("MoveToAttribute") });
Expand Down
7 changes: 4 additions & 3 deletions src/libraries/System.Private.Xml.Linq/tests/xNodeReader/IntegrityTest.cs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Test.ModuleCore;
using System;
using System.Xml;
using Microsoft.Test.ModuleCore;
using Xunit;

namespace CoreXml.Test.XLinq
{
Expand Down Expand Up @@ -250,14 +251,14 @@ public void GetAttributeEmptyNameNamespace()
public void GetAttributeOrdinal()
{
XmlReader DataReader = ReloadSource();
DataReader.GetAttribute(0);
Assert.Throws<InvalidOperationException>(() => DataReader.GetAttribute(0));
}

//[Variation("this[i]")]
public void HelperThisOrdinal()
{
XmlReader DataReader = ReloadSource();
string str = DataReader[0];
Assert.Throws<InvalidOperationException>(() => DataReader[0]);
}

//[Variation("this[name]")]
Expand Down
1 change: 1 addition & 0 deletions ...raries/System.Private.Xml.Linq/tests/xNodeReader/System.Xml.Linq.xNodeReader.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<Compile Include="ReadToNextSibling.cs" />
<Compile Include="ReadValue.cs" />
<Compile Include="XNodeReaderAPI.cs" />
<Compile Include="XNodeReaderAttributeTests.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(CommonTestPath)System\Xml\XmlCoreTest\XmlCoreTest.csproj" />
Expand Down
Loading

0 comments on commit 54f6250

Please sign in to comment.