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

Robot_Adapter_Tests: incorrect verification in PushBarsWithTagTwice() #526

Open
alelom opened this issue Aug 22, 2023 · 0 comments
Open
Assignees
Labels
size:XS Measured in seconds type:bug Error or unexpected behaviour type:question Ask for further details or start conversation

Comments

@alelom
Copy link
Member

alelom commented Aug 22, 2023

Description:

While writing some documentation, I noticed that the test method PushBarsWithTagTwice() seems to be doing a wrong verification. I believe that the assertions should be:

    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:

    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:

[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<Bar> bars1 = new List<Bar>();
    List<Bar> bars2 = new List<Bar>();
    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<Bar> pulledBars = m_Adapter.Pull(new FilterRequest { Type = typeof(Bar) }).Cast<Bar>().ToList();
    List<Node> pulledNodes = m_Adapter.Pull(new FilterRequest { Type = typeof(Node) }).Cast<Node>().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.");
}
@alelom alelom added the type:bug Error or unexpected behaviour label Aug 22, 2023
@alelom alelom changed the title Robot_Tests: incorrect verification of Robot_Adapter_Tests: incorrect verification in PushBarsWithTagTwice() Aug 22, 2023
@alelom alelom added size:S Measured in minutes type:question Ask for further details or start conversation size:XS Measured in seconds and removed size:S Measured in minutes labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XS Measured in seconds type:bug Error or unexpected behaviour type:question Ask for further details or start conversation
Projects
None yet
Development

No branches or pull requests

2 participants