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

WIP: Control flow fork #198

Closed
wants to merge 23 commits into from
Closed

Conversation

Plotist
Copy link

@Plotist Plotist commented May 15, 2018

I took @corevo 's branch as a starting point for a new way to control over test execution in order to provide control flow operators like if and while and make they inclusion as painless as possible. General plan is to complete PlaybackTree in a way where every new control flow command will have correct left and right branches pointing to the next correct command to execute. I'm also thinking about separating left, right and every possible future property related to the control flow into separate data structure bound to Command.

Basic concept of how PlaybackTreeprocesses commands is displayed in here:
fullsizeoutput_52c

With execution implemented I'll need to figure out how to properly eval conditionals in order to use stored variables as a context.

Added repeatIf, do, elseIf
Only times, continue, break left

Added test case validator class which is responsible for keeping block integrity and capable of pointing to the errors in the suite. Also added specs for it.

I'll update this description on the go.

@Plotist Plotist changed the title Control flow fork WIP: Control flow fork May 15, 2018
@@ -25,6 +25,8 @@ export default class Command {
@observable target = "";
@observable value = "";
@observable isBreakpoint = false;
@observable left = {};
Copy link
Member

Choose a reason for hiding this comment

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

Don't turn Command into an Either,
Have a Node class that has right left and command.
This information changes on a rapid basis, and is only accurate at the time of the run, which is recalculated anyway.
Keep the tree in the scope of the test run alone.

Copy link
Author

Choose a reason for hiding this comment

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

I'll take care of it!

@@ -112,6 +114,36 @@ function doCommands(request, sender, sendResponse) {
}
}

function isConditinal(commandName) {
Copy link
Member

Choose a reason for hiding this comment

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

This code is unnecessary, just add Selenium.prototype.doConditional, make it something like,
return !!eval(conditional).
This will give you more control, we just need a new class of commands, I believe.
Currently we have 2:

  • Content script commands
  • Ext commands

We also have type which is in the middle, because it's both, it has to wait like a content command, but it needs the elevated background script.
IMO just create an isFlowCommand (like isExtCommand), and execute them in that place (similarly to how I execute type, after the waiting).

Copy link
Author

Choose a reason for hiding this comment

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

This code is being reconsidered while I'm working on sandboxing.

@corevo
Copy link
Member

corevo commented May 21, 2018

We also need to understand how to handle invalid flow, we need to parse it, and error immediately when the user attempts to play.
For example:

  • If
  • While
  • Else
  • endWhile

-- this has got to fail immediately

// specific language governing permissions and limitations
// under the License.

export default class SuiteValidator{
Copy link
Member

Choose a reason for hiding this comment

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

Suite is a container of test cases,
A test case has a list of commands to run.
So in practice we validate a Test Case and not a Suite.
Let's rename this to TestCaseValidator

Copy link
Author

Choose a reason for hiding this comment

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

Sure. Makes sense

@Plotist Plotist closed this Jul 9, 2018
@Plotist Plotist deleted the control-flow branch July 9, 2018 13:34
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.

2 participants