BHoM / BHoM/Robot_Toolkit

Robot_Adapter_Tests: incorrect verification in `PushBarsWithTagTwice()`

Open
#526 0 comments 0 reactions 1 assignee Claimed by @IsakNaslundBh View on GitHub
size:XS type:bug type:question
Dominant language
C#
Stars
12
Forks
4
PR merge metrics
No merged PRs in 30d

Description

#### Description:

While writing some documentation, I noticed that the test method [`PushBarsWithTagTwice()`](https://github.com/BHoM/Robot_Toolkit/blob/5e04c82a081e3dafab3213c6f89363f0840ad3cf/.ci/unit-tests/Robot_Adapter_Tests/PushTests.cs#L127-L155) seems to be doing a wrong verification. I believe that the assertions should be:

```cs
pulledBars.Count.ShouldBe(count, "Bars storing the tag has not been correctly replaced.");
pulledNodes.Count.ShouldBe(count * 2, "Node storing the tag has not been correctly replaced.");
```

instead of:

```cs
pulledBars.Count.ShouldBe(bars.Count, "Bars storing the tag has not been correctly replaced.");
pulledNodes.Count.ShouldBe(bars.Count* 2, "Node storing the tag has not been correctly replaced.");
```

@IsakNaslundBh (the author of the test) should verify whether this assumption is correct or not?

As a side note, I believe that the structure of the test could be refactored so we can reflect the good practice of splitting in "Arrange, Act, Assert", for example as follows:

```cs
[Test]
[Description("Tests that pushing a new set of Bars with the same push tag correctly replaces previous pushed bars and nodes with the same tag.")]
public void PushBarsWithTagTwice()
{
// Arrange. Create two sets of 3 bars.
int count = 3;
List bars1 = new List();
List bars2 = new List();
for (int i = 0; i < count; i++)
{
bars1.Add(Engine.Base.Create.RandomObject(typeof(Bar), i) as Bar);
}

for (int i = 0; i < count; i++)
{
bars2.Add(Engine.Base.Create.RandomObject(typeof(Bar), i + count) as Bar);
}

// Act. Push both the sets of bars. Note that the second set of bars is pushed with the same tag as the first set of bars.
m_Adapter.Push(bars1, "TestTag");
m_Adapter.Push(bars2, "TestTag");

// Act. Pull the bars and the nodes.
List pulledBars = m_Adapter.Pull(new FilterRequest { Type = typeof(Bar) }).Cast().ToList();
List pulledNodes = m_Adapter.Pull(new FilterRequest { Type = typeof(Node) }).Cast().ToList();

// Assert. Verify that the count of the pulled bars is only 3, meaning that the second set of bars has overridden the first set of bars.
pulledBars.Count.ShouldBe(bars.Count, "Bars storing the tag has not been correctly replaced.");

// Assert. Verify that the count of the pulled nodes is only 6, meaning that the second set of bars has overridden the first set of bars.
pulledNodes.Count.ShouldBe(bars.Count * 2, "Node storing the tag has not been correctly replaced.");
}

```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.