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

Add UI/frontend test to pbench-dashboard #2980

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vishalvvr
Copy link
Member

PBENCH-800

  • Integrate WTRobot with pbench-dashboard
  • Automate valid and invalid login test cases

- Integrate WTRobot with pbench-dashboard
- Automate valid and invalid login testcases
@vishalvvr vishalvvr added Dashboard Of and relating to the Dashboard GUI testing labels Aug 24, 2022
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

This is a great start, but I think it needs to be generalized. I'd prefer that it not be tied to a particular browser (we need to test against at least Firefox and Chrome), and it definitely can't be tied to a specific server IP address and user!

.vscode/
tmp/
wtlog.log
geckodriver.log
Copy link
Member

Choose a reason for hiding this comment

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

Is there an equivalent for Chrome?

"locale": "en_US",
"log": "wtlog.log",
"dev": false
}
Copy link
Member

Choose a reason for hiding this comment

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

Trailing newlines are generally a good idea; if nothing else, it keeps GitHub from showing the big red warning icon!


> NOTE:
- Selenium_drivers folders have your selenium webdrivers geckodrivers(for firefox) and chromedrivers(for chrome and chromium)
- If script fails due to drivers issue, you need to find appropriate selenium webdriver according to your browser version(choose latest if your browser is upto to date)
Copy link
Member

Choose a reason for hiding this comment

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

s/version(/version (/ ... although perhaps the parenthesized phrase should really be a separate sentence.

s/upto/up to/

Are those version numbers browser versions or driver versions and how are they related? If they're driver versions, is there a key for relating those to browser versions?

Finally, since browser independence is going to be a strong requirement, is there a way to have the test system pre-configured for both so that we can easily wire a functional test system to be sure both run correctly?

Comment on lines +9 to +11
target: http://10.1.170.201
targets:
- http://10.1.170.201
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to hardcode IP addresses here; is there a way of importing these by variable from outside? This, in particular, is my personal test server and ideally nobody else would be depending on it at all ... in general, we'll be running our functional tests against a transient pod set up and torn down by the Jenkins pipeline and we need to run the dashboard tests against that server.

- step 3:
action: input
target: //*[@id="username"]
value: vishalvvr
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to hardcode user and password, either; and certainly not to your account! While we could stand up a CI pipeline pod with a pre-configured test user, it'd be much better to have this imported from outside via an environment variable or similar.

Copy link
Member

@npalaska npalaska left a comment

Choose a reason for hiding this comment

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

This looks good but I also concur with Dave's suggestion that we can not hardcode the server ip address and username-password like fields, we need to import that from outside, maybe a json file or something.


> NOTE

- If config(in `config.json`) is missing on initial run, tool will ask you for few configuration question and create `config.json` file.
Copy link
Member

Choose a reason for hiding this comment

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

There is no config section in config.json, right? If so maybe you can replace
config(in config.json) with just config.json


- `action`: what to perform (e.g. click, input and etc)

- `target`: on what to perform (e.g. Text widget on web page, xpath of widget and etc)
Copy link
Member

Choose a reason for hiding this comment

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

The example you showed above
target: text | xpath | css path only takes the type of the target. Is specifying only the type enough or do we need to be more verbose than that like you mentioned here?

Comment on lines +9 to +11
target: http://10.1.170.201
targets:
- http://10.1.170.201
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to specify both targets and target?

Copy link
Member

Choose a reason for hiding this comment

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

In all the steps the two keys always seem to have the same value....

@portante portante added this to the v0.72 milestone Aug 29, 2022
Comment on lines +7 to +11
```console

> pip install wtrobot

```
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the blank lines here affect the formatted result, and I don't think they make the source code easier to read; so, I recommend omitting them.

Ditto lines 27-31.

Comment on lines +13 to +22
> NOTE

- Selenium_drivers folders have your selenium webdrivers geckodrivers(for firefox) and chromedrivers(for chrome and chromium)

- If script fails due to drivers issue, you need to find appropriate selenium webdriver according to your browser version

- [firefox](https://github.com/mozilla/geckodriver/releases) & [chrome/chromium](https://chromedriver.chromium.org/downloads)

- Unzip or untar the executable and place in selenium_drivers dir.

Copy link
Member

Choose a reason for hiding this comment

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

Placing "NOTE" in a block quote is an interesting formatting choice, but placing the bulleted list under it outside the block quote is weird.

I suggest either including the bullet list in the block quote or replacing the > with bold and/or italics or with four or five-#'s.

Ditto lines 33-36.


## Executing Script

- Write all your test cases into test.yaml and execute
Copy link
Member

Choose a reason for hiding this comment

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

Put "test.yaml" in mono-space font (i.e., in back-ticks).


> NOTE

- If config(in `config.json`) is missing on initial run, tool will ask you for few configuration question and create `config.json` file.
Copy link
Member

Choose a reason for hiding this comment

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

I think the phrase, "ask you for few configuration question", should be "ask you a few configuration questions".

> NOTE

- If config(in `config.json`) is missing on initial run, tool will ask you for few configuration question and create `config.json` file.
- Make sure files which you mention as config(in `config.json`) should exist else will exit with error.
Copy link
Member

Choose a reason for hiding this comment

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

This text seems sort of obvious, so I would omit it. However, if you want to include it, perhaps phrasing like, "If any files referenced in config.json don't exist, the tool will exit with an error."


> NOTE:
- Selenium_drivers folders have your selenium webdrivers geckodrivers(for firefox) and chromedrivers(for chrome and chromium)
- If script fails due to drivers issue, you need to find appropriate selenium webdriver according to your browser version(choose latest if your browser is upto to date)
Copy link
Member

Choose a reason for hiding this comment

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

This statement seems self-evident, so I would either omit it or rephrase it along the lines of, "You must provide a Selenium webdriver which matches your choice of browser and browser version."

> NOTE:
- Selenium_drivers folders have your selenium webdrivers geckodrivers(for firefox) and chromedrivers(for chrome and chromium)
- If script fails due to drivers issue, you need to find appropriate selenium webdriver according to your browser version(choose latest if your browser is upto to date)
- [firefox](https://github.com/mozilla/geckodriver/releases) & [chrome/chromium](https://chromedriver.chromium.org/downloads)
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to have these references in their own list of one bullet. Just append them to the previous line.

- Selenium_drivers folders have your selenium webdrivers geckodrivers(for firefox) and chromedrivers(for chrome and chromium)
- If script fails due to drivers issue, you need to find appropriate selenium webdriver according to your browser version(choose latest if your browser is upto to date)
- [firefox](https://github.com/mozilla/geckodriver/releases) & [chrome/chromium](https://chromedriver.chromium.org/downloads)
- Unzip or untar the executable and place in selenium_drivers dir.
Copy link
Member

Choose a reason for hiding this comment

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

s/in selenium_drivers dir/in the `selenium_drivers` directory/

Comment on lines +4 to +9
test:
- testcase 1:
- scenario: invalid pbench-dashboard login
- step 1:
action: goto
target: http://10.1.170.201
Copy link
Member

Choose a reason for hiding this comment

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

This structure seems a little...odd. It makes reasonable sense that a test would be a sequence of "testcases". It's slightly weird that each testcase would be a mapping with a single key mapped to a sequence, although I suppose that lets you store the name of the test case as its key instead of an attribute. But, it's really weird that the first entry in the testcase sequence is a "scenario" while the other entries are "steps".

A more natural structure might look like this:

test:
- name: testcase 1
  scenario: invalid pbench-dashboard login
  steps:
  - name: step 1
    action: goto
    target: http://10.1.170.201
    ...
  ...
...

That is, test is a sequence of test cases, each of which is a mapping containing a name, scenario, and a sequence of steps; each step is a mapping containing a step name, an action, a target, and so forth.

Although, if you really like the "name attribute as key", it might look like this:

test:
- testcase 1:
    scenario: invalid pbench-dashboard login
    steps:
    - step 1:
        action: goto
        target: http://10.1.170.201
        ...
    ...
...

where test is a sequence of test cases, each of which is a mapping of a single key to a mapping containing a scenario and a sequence of steps; each step is a mapping of a single key to a mapping containing an action, a target, and so forth.

Comment on lines +9 to +11
target: http://10.1.170.201
targets:
- http://10.1.170.201
Copy link
Member

Choose a reason for hiding this comment

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

In all the steps the two keys always seem to have the same value....

@portante portante modified the milestones: v0.72, v0.73 Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dashboard Of and relating to the Dashboard GUI testing
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants