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

New Chain-Link: Pipe; Simplifying BotAction & Added New #20

Merged
merged 153 commits into from
Jul 29, 2020

Conversation

mrWh1te
Copy link
Owner

@mrWh1te mrWh1te commented Apr 1, 2020

Concept work

  • BotActionsPipeFactory
    • creates a chain-link, which is a function that returns Promise<void>
    • composes BotPipeAction functions in a pipe, similar to the main bot actions chain
      • pipes pass the resolved data of the previous function in the params of the next function
  • BotPipeActionFactory
    • returns a BotPipeAction<T>
    • generic designates the type of the value being passed along
  • BotPipeAction
    • is an async function that returns Promise<T>

Draft

  • Generic BotActionsFactory

    • can return something or not (to support outside functional call in an imperative line of code)
    • supports pipe'ing data from BotAction as an optional thing
      • if pipe'ing then use the first injects param for the piped data
        • piped data is added to end of injects
  • Basic IndexedDB BotAction's to pipe data

    • get/set from versioned db store key
    • higher order indexedDBStore()() for setting up injects for less code repeating when running multiple set/get calls
  • how do we want to normalize, map, etc these pipe's?

  • update remaining BotAction's for new interface BotAction5, then switch names

  • testing

    • add testing for pipes
    • check code comments for todo messages regarding testing

- idea is to provide new way to compose async functions in crawling the
  web that are dependent on data crawled
- idea is to rely on IndexedDB for data in crawling Instagram to reduce
  dependencies on HTML attributes, CSS classes/ID's etc in harvesting
- special BotActionsPipeFactory to create a new kind of chain of
  resolving async functions except to pass along the previous value
@mrWh1te
Copy link
Owner Author

mrWh1te commented Apr 1, 2020

What if it were possible to reuse regular BotAction functions in a pipe instead of a specific other kind

Like a higher-order function that wraps each BotAction 1 more level that takes the input of the previously resolved async function in the pipe and passes that in the injects of the next async function?

mmm

@mrWh1te
Copy link
Owner Author

mrWh1te commented Apr 1, 2020

Idea

Rewrite the bot action to return Promise<T> or Promise<T|void> (TBD if generic|void is possible)

Then handle it in the BotActionsChainFactory function. The premise is that the default nature of a BotAction is not to return anything, but can, for when it's helpful to pipe information obtained in the function (IE grabbing data from indexedDB).

So the bot actions chain factory must resolve each action with both return cases in mind.

Therefore, there is only BotAction and not the separate BotPipeAction.

Maybe the first inject of the injects is potentially the piped data? What are the issues/restrictions with that?

Michael Lage added 2 commits April 21, 2020 17:46
- pipe-able, can as a whole return something or not
- should the bot actions pipe factory be differentiated further? deprecate?
- need to expand upon the Botmation class actions() method for the simpler yet more powerful BotActionsFactory
- to continue above bullet, need to add more testing, while refocusing some deprecatabled tests to cover the BotActionsChainFactory
- deprecate the BotActionsPipeFactory ?
@mrWh1te
Copy link
Owner Author

mrWh1te commented Apr 22, 2020

If Draft 1 is to be committed

  • more testing required to cover the new functionality (optional pipe'ing)
  • need to expand upon the Botmation.actions method with new style & add testing

@codecov-commenter
Copy link

codecov-commenter commented Jun 29, 2020

Codecov Report

Merging #20 into master will decrease coverage by 42.39%.
The diff coverage is 41.74%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #20       +/-   ##
===========================================
- Coverage   94.64%   52.25%   -42.40%     
===========================================
  Files          15       25       +10     
  Lines         168      444      +276     
  Branches       23       95       +72     
===========================================
+ Hits          159      232       +73     
- Misses          9      212      +203     
Impacted Files Coverage Δ
src/botmation/actions/files.ts 0.00% <0.00%> (ø)
src/botmation/actions/indexed-db.ts 0.00% <0.00%> (ø)
src/botmation/actions/injects.ts 0.00% <0.00%> (ø)
src/botmation/actions/local-storage.ts 0.00% <0.00%> (ø)
src/botmation/actions/pipe.ts 0.00% <0.00%> (ø)
src/botmation/helpers/indexed-db.ts 0.00% <0.00%> (ø)
src/botmation/helpers/local-storage.ts 0.00% <0.00%> (ø)
src/botmation/helpers/console.ts 75.00% <33.33%> (-16.67%) ⬇️
src/botmation/helpers/pipe.ts 56.00% <56.00%> (ø)
src/botmation/actions/console.ts 60.00% <60.00%> (-40.00%) ⬇️
... and 27 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 3565910...259aa30. Read the comment docs.

- now if a botaction is used in a pipe, where previous botaction returns
  a value, but in this case, the dev is not interested in it, they can
omit, not think about it, without any breaking changes
- previously, the resolved value was injected at the beginning of the
  injects list, but that can break a botaction method signature, when
used without intending on leveraging previous returned value
- this way, we can use botactions with or without care for what is
  previously returned, given the method signature is consistent now
- BotAction now supports optional returning of values to be injected
  without breaking custom BotAction injects' signature
- waitForNavigation is no longer a higher order method to return a
  method since it does NOT customize its returned method with any params
- Proposal #11
- added defaults to generics of BotAction
- expanded upon BotAction with safe defaults as a chain link, then
  customizable pipable action
- added a few actions (to be tested, ie goBack, goFoward, savePDF)
- worked on pipe
- got IndexedDB pipeable actions working
- instagram isGuest now uses IndexedDB data
- idea of replacing options: BotOptions param in a BotAction was the
  first inject, in the injects array
- maybe replace BotOptions entirely as params, etc or MAYBE just have
  the BotAction (like output actions) designate the first inject to
being of that type?
 - having deeper BotAction types like BotFilesAction that specifies
   injects
 - getting rid of param piped from BotAction (see BotAction5), for a
   more specific BotPipeAction that specifies piped value as 1st inject
 - then a BotAction that relies on options or injects but no piped
   values can be written without any mention of
 - downside is the risk of injecting a piped value when used incorrectly
   without some kind of who is guard or branding of these actions to
know when to pipe value or inject injects, etc
- helper method to more easily strongly type your BotActions with less
  effort
- more work for same level of typing
- considering removing 'type?' from BotAction5 for instead using a
  higher-order Injects BotAction that can be customized ie IndexedDB to
set dbName, version, storename?, etc as injects for indexeddb actions,
only need strong typing at compile time
- considering new direction that may yield an easier dev path
- comments left at bottom of bot-actions.interfaces
- less is more, stick to some design constraints, and make code safer
- its weird but might work, suggests doing an object and not bothering
  with spreading, maybe keys are var names and values are corresponding
- need to try write IndexedDB()() concept, then decide
- playing around with a different kind of `injects` type instead of
  spreading an array since this is handled underneath the hood
- params are all optional, but should not all be ignored with safe
  fallbacks
- idea is you can either inject, pipe, or use the factory method to
  customize what is set in IndexedDB
- idea that a pipe is a chain-link, nothing goes in or comes out, so we
  can use a tap() botaction to pipe something from out to in
- is there a need for a pipe to return something to nonpipe? doesn't
  seem necessary
- if a pipe can take something in and return something out, then it
  would be possible to chain pipes together as a big pipe
- is that good, bad.. ?
- building, all new code being tested in examples/instagram
- indexedDBStore higher order botaction WIP
- setIKeyVal3, getIKeyVal3 methods (upgraded versions of
  setIndexDBStoreDataKeyValue, etc) are WIP
- idea for tubing in layers injects, so higher order chains/pipes
  injects are injected at the end, with most recent injected 1st
- so in theory, in the future, could expand upon higher order functions
  to support nesting
   - ie IndexedDB(dbName, dbVersion)((IndexedDBStore(Store)(...actions))
- considering ways to inject a piped value into higher order bot actions
  like indexedDBStore, maybe including it at end of injects possibly
- and add the ability to test an inject as a pipedValue, via wrapping
  the pipedValue in an obj like {brand:"piped", value:any}
- the get/set methods with BotAction5 syntax is working
- test 5 is failing, tbd
- BotAction5 > BotAction10 for now, less code in argument typing for
  majority cases
- need to add a files()() for injecting BotOptions -> BotFileOptions
- file related actions higher-order WIP, and related actions will need
  better typing
- add code to create directory if not exist, when creating/updating file?
- working higher order files()() with cookies()
- cleaned up BotActionsPipeFactory5 to become the next official one
- got started with the branding of pipedValues, as a way to
  differentiate whether last inject is a piped value, between pipes
- expanded for new Piping logging behavior and spacing vertically ie
  log() adds a line break at the end, even when given nothing to log
- refactored logPipeValue() for easier readability and improved coverage
- integration test for wait() on sleep()
- missed edge case now tested in getFileUrl()
- integration test for getting key/value from IndexedDB
- adjusted unpipeInjects() to return pipeValue at the start of the array
- added the ability to set an expected number of injects in
  unpipeInjects to fill whats missing with what w/e, default undefined
- can now strongly type the pipe value through a generic and its safer
  for situations where the number of injects is ultimately unknown
  - makes it so devs using it dont have to test their injects as much
    for more complex situations
- set method integration test
- updated the mocking of console.log and error to restore original
  functionality afterAll suite tests run
- higher order indexed-db action testing, ended up finding a bug where
  parent injects were not injected, maybe but in pipe()()() tbd
- e2e test is failing for indexed-db on an error, reported on github
  jest-puppeteer, linked in comments
- recently added type gaurds unit testing
- unit-testing chainRunner, pipeRunner, and pipeActionOrActions
- unit testing chain()
- pipe() unit-testing covers all cases
- assemblyLine() unit-testing
- missed coverage testing in chain()
- added additional constraint for chain() that the awaited result is no
  other than undefined
- chainRunner() extended testing to add constraint that a resolved
  chainRunner is no other value than undefined
- removed tests for bot-actions-chain.factory
- deleted bot-actions-chain and /factories directory, no longer needed -
  such functionality is now available as an assembly-line BotAction
- indexedDBStore() runs an assembly-line so needs to await inject()()()
- remove dead url
@mrWh1te
Copy link
Owner Author

mrWh1te commented Jul 29, 2020

As of a few commits ago, the minimum threshold for testing coverage in this Project has been met

As of now, only 1 helper file has lines of code not covered

Close to 100%:
testing coverage report

@mrWh1te
Copy link
Owner Author

mrWh1te commented Jul 29, 2020

Before merge:

  • clean up code for outdated comments, etc
  • remaining indexeddb helper functions testing, 100% test coverage

Next PR:

  • updating the build
  • update Puppeteer dependency
    • which versions supported (2+ ?)
    • commented out indexeddb actions e2e test, has the relevant issue been resolved?
  • npm release v2
  • updating the docs

Future:

  • unit test for deeply composed BotAction's ie does injects from parent make it all the way to bottom

- verifying function running of all BotAction's in chainRunner()
- missed getPage() on class.spec
- expanding upon testing of indexed-db helpers, refining their
  functionality
- missed edge case in asyncConstructor for class.spec
- unit testing getIndexedDBStoreValue(), seems I have more to understand
  with IndexedDB's nature as the onerror handlers didn't work as expected
- added the ability to not specify the database version number since not
  providing it results in a safe route of latest version, helpful
- updated relevant testing
- the injects params and higher order params order for IndexedDB
  BotAction's have changed in response to this
- had to ignore a line of code, but left an assertion that covers it,
  but for whatever reason, the coverage report misses it
- the code is the ability to create a new store during a db version
  upgrade (version number bigger than before), tested with comments
@mrWh1te
Copy link
Owner Author

mrWh1te commented Jul 29, 2020

Test coverage is now 💯🥳
test coverage 100

This was referenced Jul 29, 2020
- separate test file to test an inject()() theory about nesting since
  this test requires assemblyLine() not mocked, it was separated
- can put other theories of nesting, etc in here
- might move in future, reconsolidate
- reviewed todo's in comments
- some minor cleaning
@mrWh1te mrWh1te merged commit 840e9d7 into master Jul 29, 2020
@mrWh1te mrWh1te deleted the bot-actions-pipe-experiment branch July 29, 2020 21:18
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