Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

Selenium test for WebUI #398

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Selenium test for WebUI #398

wants to merge 4 commits into from

Conversation

gz
Copy link
Contributor

@gz gz commented May 5, 2023

No description provided.

gz added 3 commits May 2, 2023 23:21
So we can refer to them in integration/selenium tests.
We properly update the query cache after a mutation.
This helps avoid races where client serves stale data
after we invalidate the query.
- A framework for writing selenium test against the WebUI.
- Also contains an initial test for the UI.
- Brings various annotations for IDs in critical UI elements
  to simplify testing.
@gz gz temporarily deployed to github-pages May 5, 2023 07:41 — with GitHub Actions Inactive
@gz gz temporarily deployed to github-pages May 5, 2023 08:01 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented May 5, 2023

Benchmark results

Nexmark

  • 1 out of 21 queries have regressed ❗
  • Compared results from dbe4663 (main) with a356006 (PR)
    No benchmark results found for current main revision, compared against dbe4663
name main~70 [kOp/s] PR [kOp/s] Tput change [%] Assessment Peak RSS diff
q0 5262.63 5212.89 -1 ✔️ 119.3 MB
q1 5429.6 5165.72 -5 ✔️ 79.8 MB
q2 5361.6 5334.28 -1 ✔️ 50.0 MB
q3 5321.94 5352.24 1 ✔️ 48.2 MB
q4 4802.55 4766.7 -1 ✔️ -180.1 MB
q5 5378.33 5277.86 -2 ✔️ -180.1 MB
q6 4880.13 4366.42 -11 🔻 -111.4 MB
q7 2559.31 3638.93 42 🌲 -4.8 GB
q8 5161.2 5049.11 -2 ✔️ -4.8 GB
q9 875.448 870.349 -1 ✔️ -270.3 MB
q12 4910.28 5005.27 2 ✔️ -270.3 MB
q13 3645.17 3584.27 -2 ✔️ -270.3 MB
q14 5481.43 5461.3 0 ✔️ -270.3 MB
q15 4895.11 5422.39 11 🌲 -270.3 MB
q16 1045.16 1039.69 -1 ✔️ -270.3 MB
q17 3134.56 3282.9 5 ✔️ -201.1 MB
q18 1440.13 1469.76 2 ✔️ 942.1 kB
q19 1435.15 1377.41 -4 ✔️ 942.1 kB
q20 1500.83 1575.19 5 ✔️ 942.1 kB
q21 5117.36 4927.48 -4 ✔️ 942.1 kB
q22 5356.59 5444.81 2 ✔️ 942.1 kB

Galen

  • Compared results from dbe4663 (main) with a356006 (PR)
    No benchmark results found for current main revision, compared against dbe4663
name main~70 [s] PR [s] Runtime change [%] Assessment
galen 28.6952 28.9488 1 ✔️

LDBC

  • 1 out of 4 queries have regressed ❗
  • Compared results from dbe4663 (main) with a356006 (PR)
    No benchmark results found for current main revision, compared against dbe4663
algorithm dataset threads main~70 [kEVPS] PR [kEVPS] Tput change [%] Assessment Peak RSS diff
bfs graph500-22 1 1835.7 1764.56 -4 ✔️ 69.6 kB
bfs datagen-8_4-fb 6 8173.61 7502.95 -8 🔻 31.7 MB
pagerank graph500-22 1 681.655 690.726 1 ✔️ 32.8 kB
pagerank datagen-8_4-fb 6 1994.79 2041.9 2 ✔️ -320.5 MB

Nexmark (with Persistence)

  • Compared results from dbe4663 (main) with a356006 (PR)
    No benchmark results found for current main revision, compared against dbe4663
name main~70 [kOp/s] PR [kOp/s] Tput change [%] PR DRAM [kOp/s] DRAM diff [%] Assessment
q0 2399.23 2384.59 -1 2401.69 -1 ✔️
q1 1713.93 1641.45 -4 1722.54 -5 ✔️
q2 2407.67 2327.45 -3 2164.82 8 ✔️
q3 2036.93 2008.32 -1 2262.34 -11 ✔️
q4 360.256 359.343 0 1436.3 -75 ✔️
q5 2015.3 1990.09 -1 2313.16 -14 ✔️
q6 332.421 337.382 1 1375.07 -75 ✔️
q7 639.584 663.936 4 1332.5 -50 ✔️
q8 2171.19 2192.41 1 2277.02 -4 ✔️
q9 79.0764 79.3372 0 390.792 -80 ✔️
q12 838.481 851.714 2 1833 -54 ✔️
q13 443.491 441.395 0 1004.26 -56 ✔️
q14 1697.52 1653.07 -3 1717.06 -4 ✔️
q15 198.112 197.726 0 1231.74 -84 ✔️
q16 25.5771 25.8495 1 284.255 -91 ✔️
q17 81.4521 80.614 -1 812.167 -90 ✔️
q18 124.763 124.37 0 807.769 -85 ✔️
q19 182.489 179.965 -1 655.318 -73 ✔️
q20 505.761 497.938 -2 966.35 -48 ✔️
q21 1489.8 1486.49 0 1504.32 -1 ✔️
q22 2102.89 2051.11 -2 2109.1 -3 ✔️

@gz gz requested a review from mihaibudiu May 8, 2023 04:35
@gz gz marked this pull request as ready for review May 8, 2023 04:35
/// This is the SQL code that will be saved.
///
/// I noticed that if you put newlines immediately after the braces, the code
/// editor might add another brace to complete it, and then the code won't be

Choose a reason for hiding this comment

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

that's a stupid editor. should have an option not to do that.


/// A config for a HTTP endpoint.
///
/// The \x08 is a backspace character, which is needed to remove the space that

Choose a reason for hiding this comment

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

sick

<PageHeader
title={<Typography variant='h5'>Pipeline Creator</Typography>}
subtitle={<Typography variant='body2'>Define an end-to-end pipeline with analytics.</Typography>}
/>
<Grid item xs={12}>
{/* id referenced by webui-tester */}

Choose a reason for hiding this comment

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

why is the comment in braces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.jsx/.tsx requires this -- you can't just put /* */

See: https://www.educative.io/answers/how-to-add-comments-in-jsx

let caps = DesiredCapabilities::chrome();
let driver = WebDriver::new("http://localhost:9515", caps).await?;

let timeouts =

Choose a reason for hiding this comment

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

describe the three fields?

TimeoutConfiguration::new(None, None, Some(std::time::Duration::from_millis(2000)));
driver.update_timeouts(timeouts).await?;

driver.goto("http://localhost:3000").await?;

Choose a reason for hiding this comment

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

where do all these ports come from?
should they be in a config file?


/// Set the name of the program.
pub async fn set_name(&self, name: &str) -> WebDriverResult<()> {
let elem = self.form_name.resolve().await?;

Choose a reason for hiding this comment

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

this kind of code is repeated over and over, wonder whether it could be a utility function.

/// Wait until `save_status` reaches "SAVED".
///
/// Return an error if it doesn't reach "SAVED" within the given timeout.
pub async fn wait_until_saved(&self, timeout: Duration) -> Result<()> {

Choose a reason for hiding this comment

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

this function is also very similar. perhaps a common trait?

}
return Err(anyhow!("`table` not found in program tables"));
} else {
return Err(anyhow!("`connector_name` not found in inputs"));

Choose a reason for hiding this comment

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

If the error branch is shorter I usually prefer to handle it first and have no 'else'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually like the else branch, shows that both outcomes are handled with minimal cognitive load. Every time there is no else branch I worry...

I didn't make this up, I follow suggestions from Code Complete (2nd ed, chap 15) which says:

  • Write the nominal path through the code first; then write the unusual cases
  • Consider the else clause If you think you need a plain if statement, consider
    whether you don’t actually need an if-then-else statement. A classic General Motors
    analysis found that 50 to 80 percent of if statements should have had an else clause
    (Elshoff 1976).


/// Find the pipeline identified with `name` and press pause.
pub async fn pause(&self, name: &str) -> Result<()> {
let row_idx = self.find_row(name).await?;

Choose a reason for hiding this comment

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

factor out first two lines in a separate function?

}

impl PipelineDetails {
pub async fn get_records_for_table(&self, table_name: &str) -> Result<usize> {

Choose a reason for hiding this comment

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

what is the result?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants