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

abort() - Abort Signal aka break #66

Merged
merged 22 commits into from
Sep 4, 2020
Merged

abort() - Abort Signal aka break #66

merged 22 commits into from
Sep 4, 2020

Conversation

mrWh1te
Copy link
Owner

@mrWh1te mrWh1te commented Aug 31, 2020

For #36 with consideration to #38 & #61

Break is a reserved keyword, so went with abort()

This BotAction returns a signal to be processed by its assembler. The purpose is to inform the assembler that it needs to stop running its assembled BotAction's and possibly return a Signal (with assembledLines decreased by 1 count) so the higher-order assembler can abort and follow through so anywhere in the program, or fully abort with (assembledLines set to 0)

  • abort()
    • helpers
    • interface
    • tests
    • examples
      • post-pone examples for docs, since a good understanding example will be best in the docs in a new Advanced topic about aborting which goes into the BotAction but really you don't need the abort() BotAction to use this feature, you simply need to return an AbortLineSignal, so an example in the docs of a forAll() which uses custom BotAction's that check websites for things and aborts if they don't meet some criteria (1 line of aborting), which won't break the loop but the one loop line of assembly for the specific loop iteration
  • Update assemblers to support aborting
    • chain()
    • pipe()
    • pipeActionOrActions()
    • assemblyLine()
    • pipeRunner()
    • chainRunner()
  • Update Utilities to support aborting
    • givenThat
    • forAll
    • doWhile
    • forAsLong
  • Update to support aborting
    • inject
    • errors
  • 1 unit theory test e2e'ish to confirm actual results of aborting multiple lines but not infinite
  • 1 unit theory test e2e'ish to confirm actual results of aborting infinite (pipe value returned?)
  • verify proper assembledLines counts cases with tests

To be included in next minor release v2.1.0

- interface, helper function, and BotAction to return a signal for
  aborting - concept
@codecov
Copy link

codecov bot commented Aug 31, 2020

Codecov Report

Merging #66 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #66   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        25    +3     
  Lines          472       538   +66     
  Branches       106       135   +29     
=========================================
+ Hits           472       538   +66     
Impacted Files Coverage Δ
src/botmation/actions/abort.ts 100.00% <100.00%> (ø)
src/botmation/actions/assembly-lines.ts 100.00% <100.00%> (ø)
src/botmation/actions/errors.ts 100.00% <100.00%> (ø)
src/botmation/actions/files.ts 100.00% <100.00%> (ø)
src/botmation/actions/inject.ts 100.00% <100.00%> (ø)
src/botmation/actions/utilities.ts 100.00% <100.00%> (ø)
src/botmation/helpers/abort.ts 100.00% <100.00%> (ø)
src/botmation/types/abort-signal.ts 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85cbe38...76e8dd9. Read the comment docs.

- given the tight simple use of the helper function and since its
  unit-tested, we can simply integrate test this botaction
@mrWh1te
Copy link
Owner Author

mrWh1te commented Aug 31, 2020

Helper function unit-tested, BotAction integration tested.

Didn't unit test the BotAction since by integration testing it with the helper function, and its unit-testing, we have strong coverage for these pieces of code.

- updated testing to include coverage for new functionality
- coverage report was producing a false negative with not covering a
  partcular branch case (assembledLines > 1) but there is a test for it
@mrWh1te mrWh1te mentioned this pull request Aug 31, 2020
50 tasks
@mrWh1te mrWh1te mentioned this pull request Aug 31, 2020
@mrWh1te
Copy link
Owner Author

mrWh1te commented Aug 31, 2020

processAbortSignalReturn function?

Something for the repeated code inside the if isAbortLineSignal() blocks

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 1, 2020

Important to note

  • This PR changes the default generics of a BotAction from returning Promise<void> to Promise<void|AbortLineSignal>

All of botmation's core assembly-lines will support this signal with a minor difference in switchPipe() for the switch/case flow.

Note, all Utilities use Pipe for assembled BotAction's, therefore will support the AbortLineSignal too

Custom assemblers that don't implement the aborting functionality themselves will break the system's flow so a note in future tutorial about building assemblers needs to include a section about the Abort Line Signal

- adjusted to be chain friendly, as much as possible so they dont look
  to return a pipe value, but might return an AssemblyLineSignal
@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 1, 2020

The resolving of a ConditionalBotAction in a Utility BotAction has not been setup to abort the botaction if that signal is returned instead of boolean

edit:
for now, ConditionalBotAction's won't be updated to return a AbortLineSignal therefore Utility BotAction's won't have to parse the end result of a ConditionalBotAction for an AbortLineSignal. If it's needed in the future, it can be added, but this PR is getting big with wide scope

edit2: going to include the conditionalbotaction's as a means to abort to keep things consistent. Adds more work than originally planned, but important to this functionality

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 1, 2020

To get around the fact that Utility functions don't return PipeValue's to be chain friendly, in the event that the AbortLineSignal aborts inside a Utility BotAction (assembledLines = 1), then it will go ahead and return the pipeValue from the AbortLineSignal object.

To get around the conflicting types and reduce work, the value is being typed as an AbortLineSignal

Not ideal, but works for now. The value is ignored by chain anyway, unless it's an AbortLineSignal so it doesn't break the code but the typing isn't perfect, it's hacky for this piece of functionality when it comes to typing the returned value from these functions when ran inside a pipe versus inside a chain

edit: I don't want to detect injects for a pipe to base the return on that, since it hurts the ability to use that BotAction's as a Bot to return a value without manually injecting a pipe object. What I'm doing is, if the types aren't happy because chain doesn't want values that are not typed AbortLineSignal's, then don't worry about it because chain ignores values that are not of that type anyway. The immediate down side is that if a Utility BotAction aborts with one level of assembly with a pipe value, that pipe value will be "typed" as an AbortLineSignal... but since assemblers check the return value anyway with isAbortLineSignal(), it corrects the typing in time so it's not bad, but not perfect

edit2: perfect would be to get rid of chain, or maybe chain doesn't support the AbortLineSignal feature.. but in the end, I believe this solution will suffice

- testing for AbortLineSignal
- expanded givenThat to handle AbortLineSignal from the conditional
  botaction
- going to support AbortLineSignal in resolution of a single
  ConditionalbotAction, so more work like assemblers for Utilities
- conditional handles abort signal
- assembled actions handle abort signal
- with unit-tests
- if you're going to abort in a conditional, versus in assembled of a
  givenThat, there is a difference in assembly line levels since the
  conditional is treated like the givenThat botaction but the assembled
  botaction's ran when the conditional passes is treated as a sub-line
@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 1, 2020

With how the AbortLineSignal is implemented in Utility BotAction's that loop, it's possible to abort a loop iteration, but still continue the loop since each iteration is a sub-line, you can abort 1 assembledLines, so just the loop iteration (the assembled line). If you want too, from an assembled botaction, in a looping utillity botaction, abort out of the loop too, you need to abort by 2, at least (1 for the assembled line, and 1 for the botaction itself, the loop botaction)

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 2, 2020

Wondering about a new reset()() BotAction

To catch an "AbortLineSignal" with its pipeValue to restart a line of BotAction's with the new pipe value....

edit:
could be similar to errors()() with block scope naming so when it happens, it can log an informational message to console with block name (ie which line reset) and pipeValue (from the AbortLineSignal object)

edit2:
maybe reset()() uses errors()() ? Reset runs in a loop until the final BotAction assembled returns anything but an AbortLineSignal

therefore, need 1 more botaction to focus on reverting an abortlinesignal into something more constructive, could abstract it away as some kind of signal receiver which can than operate on it through some kind of cases based on signal types?

edit3:
recycle()() for catching AbortLineSignal's ? Leave errors()() out of this so people can combine them as necessary

edit4:
it would be nice to have one BotAction that does both error catching and abortlinesignal recycling with ONE block name

edit5:
imagine:

await recycle('block A')( // recycle will pipe in the value from the AbortLineSignal.pipeValue property
    switchPipe('initial-run')(
       case('initial-run')(
          // initial run that might return an AbortLineSignal
       ),
       abort(), // equivalent to break; - special case when ran inside switchPipe()()
       case('abort line signal pipe value when something wasn't found')(
          // look somewhere else for it
        ),
       abort(), 
       // other cases
       case()(
           // default for unhandled AbortLineSignal.pipeValue cases
       )
   )
)(page)

edit6:
might be easier without switchPipe...

await pipe('initial-run')(
   recycle('block A')( // recycle will pipe in the value from the AbortLineSignal.pipeValue property
       case('initial-run')(
          // initial run that might return an AbortLineSignal
          // basically the "main" script of the bot that initially runs
          // idea is when bot runs into a condition it cannot handle, it can try again differently
             // by returning an AbortLineSignal with a pipeValue that matches a case below
       ),
       case('abort line signal pipe value when something wasn't found')(
           // look somewhere else for it
           // similar to intial run, but slightly different given the pipeValue form the AbortLineSignal
       ),
       // other cases
       case()(
           // default for unhandled AbortLineSignal.pipeValue cases
           // however in this case, it will run after the initial-run too......
       )
   )
)(page)

reminds me of a 🪀 yo-yo

edit7:
Maybe recycle by default does not recycle the infinite signal of abort(0) unless specified ? A way to bypass recycle() with some kind of Abort Line Signal pipeValue

- condition and assembled botactions
- with unit testing
- in assembly and condition
- with unit-testing
- added testing that covers the integration with pipe for handling array
  of of botactions and unit-tested the single action results handled by
  pipeActionOrActions
- unblocks forAll()() from completion
- chainRunner will return AbortLineSignal.pipeValue when it's the last
  aborted BotAction line
- assemblyLine running a chain will return AbortLineSignal.pipeValue
  when it's the last aborted Botaction line
- chain will not return AbortLineSignal.pipeValue when it's the last
  aborted botaction in line
- this creates a series of steps to shift in functional complexity from
  chain -> assemblyLine (as chain) -> assembyLine(true) (as pipe) -> pipe
- unit-testing
@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 2, 2020

Chains, even if last aborted, will not return AbortLineSignal.pipeValue

ChainRunner will though, it is BotAction<void|AbortLineSignal|PipeValue> where the case of PipeValue type is from the Abort Line Signal object.

Assembly Line, if running as a chain, will return pipeValue from Abort Line Signal object. When running as pipe, it returns pipeValue.

Pipe always returns pipeValue, even if empty (then undefined)

This gives new devs a stepping ladder for embracing these concepts ie:
chain() -> assemblyLine() -> assemblyLine(true) -> pipe()
no abort line signal value return -> return abort line signal pipe value -> return abort line signal pipe value + piping -> officially piping

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 3, 2020

Need to update Utilities a bit

Decided to go with 2 levels of assembledLines to abort to completely break out of these functions.

1 -> the assembled line itself (maybe iterating in a loop or evaluating the condition/handling the child-line)
2 -> the botaction itself that is running the loop or the condition is the 2nd level

Therefore it's consistent to break out of utility botaction's level wise, right now breaking out of (1) will break out into the higher-context, skipping (2)

The past thinking was the BotAction itself counts as 1 level/1 line, but in reality it's not what's happening. So forAll()() will starts its first iteration with an assembled line that may abort out, but then the loop itself should continue if the abort was just 1 assembledLine, as the BotAction itself is on a higher line, therefore to break out of the entire forAll()() you would need at least abort(2)

edit:
This is different then assembly-lines, since they are considered 1 line each, they don't have sub-lines. Even assemblyLine()() that uses pipeRunner/chainRunner, is considered one line itself as the BotAction itself represents 1 line

edit2:
which is why errors()() and inject()() will have to count as two levels to break out of the line/botaction of error handling or new injects injecting

edit3:
it's the whole coffee shop - alice in wonderland metaphor. As a line forms of people, each person is a BotAction, but if it's an Assembly Line, then that person is actually a whole other line of people, a (sub/child)-line. Errors/inject/utilities they are botaction's with child-lines that they operate through an assembly line, by adding some unique piece of functionality - that's the additional line, even if 1 person (ie errors)

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 3, 2020

forAll()() => 2 levels of assembly (loop iteration, looping) to fully abort
givenThat()() => 1 assembled line (if condition -> assembly line) to fully abort

These take abort(2) to fully abort, UNLESS the condition itself aborts which only needs 1 to abort
forAsLong()() => 2 levels of assembly (looping, condition/loop iteration) to fully abort
doWhile()() => 2 levels of assembly (looping, condition/loop iteration) to fully abort

That allows the dev to abort() an iteration, but keep the loop going from within a loop iteration, similar to forAll()() except it doesn't evaluate a condition each time that can abort

- processAbortLineSignal() helper unit-tested
- need to update utilities and others to use the helper to reduce
  testing code
- tests are passing, coverage is passing
@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 4, 2020

Introduced processAbortLineSignal() to reduce duplicated code

Utilities and Assembly Lines are updated with correct assembly lines aborting (count) and are using processAbortLineSignal()

- it is going to be treated as one assembly line, similar to givenThat
- therefore update was small, just return chain resolved result
@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 4, 2020

errors needed a little tweak and inject is fine

They are similar to givenThat, where it's 1 line of functionality that its ran, but more similar to an assembler for assembling these actions to run once except with some kind of added unit of functionality

Since there is no granular control to gain from requiring 2 lines of assembly to abort an errors or inject, it was left as is

@sonarcloud
Copy link

sonarcloud bot commented Sep 4, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 1 Security Hotspot to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mrWh1te
Copy link
Owner Author

mrWh1te commented Sep 4, 2020

abort example for future docs:

await forAll(websites)(
  website => ([
     goTo(website),
     givenThat(websiteIsntVisible(website))(
        abort(2) // abort givenThat and the forAll loop iteration for this website, but not the loop
                     // for the loop, that would take at least 3
     )
  ])
)(mockPage)

need a better condition

@mrWh1te mrWh1te changed the title Abort Signal abort() - Abort Signal aka break Sep 4, 2020
@mrWh1te mrWh1te merged commit aa4fea6 into master Sep 4, 2020
@mrWh1te mrWh1te deleted the abort-signal branch September 4, 2020 19:08
@mrWh1te mrWh1te added this to the v2.1 milestone Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant