-
Notifications
You must be signed in to change notification settings - Fork 3
Docs for saveState and restoreState #89
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
base: master
Are you sure you want to change the base?
Conversation
c601228
to
134cf1d
Compare
134cf1d
to
7e531b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's fix typos and add command params tables
docs/commands/browser/saveState.mdx
Outdated
|
||
## Usage {#usage} | ||
|
||
Command return state dump from page, it will include cookie, localStorage and sessionStorage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command returns
It will include cookies
etc. I think it would be good to let some LLM scan this for typos and improve wording
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any changes in this sentence, it still has mistakes, isn't it?
|
||
## Использование {#usage} | ||
|
||
Команда вернёт объект включающий cookie, localStorage и sessionStorage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice to have "Command Parameters" table like we do on other pages, e.g. https://testplane.io/docs/v8/commands/browser/setMeta/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
|
||
## Использование {#usage} | ||
|
||
Вы можете восстановить данные из файла (используя параметр `path`) или из объекта (используя параметр`data`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of typos here e.g. "нуд=жные" and I'd use more structured language here. Again, I'd just use LLM to improve spelling and wording here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
✅ Successfully deployed static |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A great result overall! 🔥
Though I think at least some of the issues I raised need to be addressed, because docs are the face of the product
|
||
Browser command that restores session state (cookies, local and session storages) from a file or variable. | ||
|
||
<Admonition type="warning"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this warning still needed?
docs/commands/browser/saveState.mdx
Outdated
|
||
## Usage {#usage} | ||
|
||
Command return state dump from page, it will include cookie, localStorage and sessionStorage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any changes in this sentence, it still has mistakes, isn't it?
docs/commands/browser/saveState.mdx
Outdated
Command return state dump from page; it will include cookie, localStorage, and sessionStorage. | ||
But using params, you can disable some data. | ||
Also, if you provide the `path` param, you can get a dump in a file. | ||
After saving the state, you can use it in the [restoreState](../restoreState) command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides spelling mistakes, the way this section is structured makes it hard to follow.
I strongly suggest to use LLM to improve this section. For example, I used this prompt:
Improve spelling and wording of this documentation section. Make it easy to follow and clear. Respond only with markdown of corrected doc section.
And got:
This command returns a snapshot of the page state, including cookies, localStorage, and sessionStorage.
You can use parameters to exclude specific types of data if needed.
If you provide the path parameter, the state dump will be saved to a file.
The saved state can later be restored using the restoreState command.
```typescript | ||
import type { SaveStateData } from "testplane"; | ||
|
||
const stateDump: SaveStateData = await browser.saveState({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really nice to briefly comment what's exactly inside that SaveStateData object. Or, if the SaveStateData interface is simple enough, we can include it right in the example.
docs/commands/browser/saveState.mdx
Outdated
<tr><td>**Name**</td><td>**Type**</td><td>**Description**</td></tr> | ||
</thead> | ||
<tbody> | ||
<tr><td>path</td><td>String</td><td>Path to file where state will be saved.</td></tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use Typescript-style types here and wrap them in backticks: string
, boolean
, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In another docs tables we have this style, but ok here I changed to typescript style
You can restore state from file (using the `path` param) or from object (using the `data` param). | ||
But if you provide both, the file will have the highest priority. | ||
Also, you can provide `cookies`, `localStorage` and `sessionStorage` params to restore only what you need. | ||
Data for restore state you can get from the [saveState](../saveState) command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as well spelling and wording needs to be improved (see comment below). IMO starting sentences from "but" and "also" doesn't quite fit the docs writing style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
docs/config/after-all.mdx
Outdated
Here is an example of suing this hook for logout. | ||
|
||
```typescript title="testplane.config.ts" | ||
import { launchBrowser } from "testplane/unstable"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this example is useless in practice. No one would want to restore tests after all tests are done executing.
A more useful example would be some kind of cleanup, like stopping a server, restoring files, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to example where we remove state file
docs/config/after-all.mdx
Outdated
|
||
## Usage Example {#example} | ||
|
||
Here is an example of suing this hook for logout. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typos
I strongly suggest to use either LLM or some editor to auto-detect and fix them. There are a lot of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments from english versions of these articles apply to ru localization as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
bc2dbda
to
92087ad
Compare
No description provided.