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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a randomized test for the variable-looping-vus requirements #1413

Merged
merged 2 commits into from
Apr 27, 2020

Conversation

na--
Copy link
Member

@na-- na-- commented Apr 27, 2020

I gave up on completely understanding all of the corner cases of #1392, or on simplifying it... for now... 馃槄 But here's an extra test that hopefully validates that however we segment a variable-looping-vus executor, it will always execute what it would have executed if it was running on a single machine.

Whether that execution is completely correct is another topic 馃槃 My manual tests didn't find any issues there, but I would still want some easier to follow implementation eventually, maybe when we adapt it to data partitioning.

@na-- na-- requested review from mstoykov and imiric April 27, 2020 13:39
Copy link
Collaborator

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

I have some readability comments, but otherwise looks Okay ;)

I was thinking of doing it by adding up all the segments steps .. but this is probably better and seems simpler then what I thought adding up will be :D

@na--
Copy link
Member Author

na-- commented Apr 27, 2020

I was thinking of doing it by adding up all the segments steps

This would definitely be more complicated. I was going to refactor GetFullExecutionRequirements(), to use it here as well, but it ended up nasty, and wasn't worth it just for this test, so I sort of implemented the reverse of it, which was much simpler.

@na-- na-- requested a review from mstoykov April 27, 2020 14:34
@na--
Copy link
Member Author

na-- commented Apr 27, 2020

@mstoykov I think we should first merge this in your branch, despite the unrelated failing CDNJS test, and then you can rebase on new-schedulers, where the test has been silenced?

@na-- na-- merged commit 4ac0dec into varriableLoopingVUsStepping Apr 27, 2020
@na-- na-- deleted the varriableLoopingVUsSteppingTest branch April 27, 2020 14:45
ci++
}
}
t.Logf("Subtracting %d VUs (out of %d) at t=%s", sub, p.PlannedVUs, p.TimeOffset)
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is very noisy when run with -v, which I usually enable to see the full output, but now this test prints ~80K lines, forcing me to use tee, etc. :-/

Can we use a debug logger here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am for commenting all the Logf lines and leaving only the random seed one as that is needed to get it reproduced - if you get an error on this, trust me you would want to be able to reproduce it ... the size of the output is way too big to just read it in circleci for example

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.

None yet

3 participants