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

Adding end-to-end test for the QLever UI #56

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .github/workflows/ui-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: UI check

on:
push:
branches: [ master ]
pull_request:
branches: [ master ]
merge_group:

jobs:
build:
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v3
- name: Install dependencies
run: |
# See https://docs.docker.com/engine/install/ubuntu/
sudo apt install -y apt-transport-https ca-certificates curl software-properties-common
sudo apt remove docker docker-engine docker.io containerd runc
curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo apt-key add -
sudo add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu focal stable"
sudo apt update
sudo apt-cache policy docker-ce
sudo apt install docker-ce containerd.io
# Install packages for use of Selenium
pip3 install selenium
Copy link
Contributor

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 better if selenium was part of requirements.txt

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps a requirements-test.txt for test requirements. This way users only have to install the necessary packages but the testing requirements are still much more visible.


- name: Build docker image for QLever UI
run: sudo docker build -t qlever-ui ${{github.workspace}}

- name: Run docker image for QLever UI
run: docker run -it -d -p 7000:7000 -v $(pwd)/db:/app/db --name qlever-ui qlever-ui

- name: Test the QLever UI using Selenium
run: python3 end2end-test.py --url http://localhost:7000

- name: Remove the docker image
run: docker rm -f qlever-ui
Binary file modified db/qleverui.sqlite3
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change intended?

Binary file not shown.
157 changes: 157 additions & 0 deletions end2end-test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
#!/usr/bin/python3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#!/usr/bin/python3
#!/usr/bin/env python3

So it works with virtualenvs

"""
Copyright 2023, University of Freiburg,
Chair of Algorithms and Data Structures
Author: Hannah Bast <[email protected]>
"""

from selenium import webdriver
from selenium.webdriver.firefox.options import Options
from selenium.webdriver.firefox.firefox_binary import FirefoxBinary
# from selenium.webdriver.chrome.options import Options
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused import

from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as EC

from selenium.webdriver.common.by import By

import argparse
import logging
import sys


# Global log with custom formatter, inspired by several posts on Stackoverflow.
class MyFormatter(logging.Formatter):
Copy link
Contributor

Choose a reason for hiding this comment

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

MyFormatter should be renamed to something that doesn't scream coding tutorial :D

def __init__(self):
super().__init__(datefmt="%Y-%m-%d %H:%M:%S")

def format(self, record):
format_orig = self._style._fmt
fmt_begin, fmt_end = "", ""
if record.levelno == logging.ERROR:
fmt_begin, fmt_end = "\x1b[31m", "\x1b[0m"
elif record.levelno == logging.WARN:
fmt_begin, fmt_end = "\x1b[35m", "\x1b[0m"
fmt = "%(asctime)s.%(msecs)03d %(levelname)-5s %(message)s"
self._style._fmt = fmt_begin + fmt + fmt_end
result = logging.Formatter.format(self, record)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just a less readable way of

Suggested change
result = logging.Formatter.format(self, record)
result = super().format(record)

self._style._fmt = format_orig
return result


log = logging.getLogger("e2e test logger")
log.setLevel(logging.INFO)
handler = logging.StreamHandler()
handler.setFormatter(MyFormatter())
log.addHandler(handler)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest you move this inside the if __name__ == '__main__' block.
This also makes clear that the level is set twice



class QleverUiTester:
"""
Class or testing the Qlever UI.

NOTE: The basic structure of this code is taken from
https://github.com/ad-freiburg/hisinone-scraper
"""

def __init__(self, headless, url, num_retries):
"""
Basic settings and open the browser window.
"""

self.headless = headless
self.url = url
self.num_retries = num_retries
self.timeout_loading = 5
options = Options()
if self.headless:
log.info("Running in \x1b[1mheadless\x1b[0m mode")
options.add_argument("-headless")
else:
log.info("Not headless, rerun with --headless to activate")
log.info("Initializing webdriver ...")
# options.binary = FirefoxBinary("/usr/bin/firefox")
self.driver = webdriver.Firefox(options=options)
# self.driver = webdriver.Chrome(options=options)
# self.driver.set_window_position(100, 0)
# self.driver.set_window_size(1400, 600)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the unused code


def done(self):
"""
Close the browser window if it's still there.
"""

try:
self.driver.close()
except Exception:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would we ignore any exceptions here?


def test(self):
"""
Some basic tests to check if the UI is working.
"""

for i in range(self.num_retries):
try:
self.driver.get(self.url)
WebDriverWait(self.driver, self.timeout_loading).until(
EC.presence_of_element_located((By.ID, "query")))
log.info(f"Page {self.url} loaded successfully")
break
except Exception as e:
if i < self.num_retries - 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if I like this retry mechanism. Ideally the timeout should be long enough so that it always works. If it casually fails that just means the timeout is too slow in my opinion

log.info(f"Loading page failed"
f" (attempt {i + 1} of {self.num_retries}"
f", error: \"{str(e)}\")"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
f", error: \"{str(e)}\")"
f", error: \"{e}\")"

f", trying again ...")
else:
log.error("Aborting after %d retries." % self.num_retries)
self.done()
sys.exit(1)


class MyArgumentParser(argparse.ArgumentParser):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, the class should be renamed

"""
Override the error message so that it prints the full help text if the
script is called without arguments or with a wrong argument.
"""

def error(self, message):
print("ArgumentParser: %s\n" % message)
self.print_help()
sys.exit(1)


if __name__ == "__main__":

# Setup parser and basic usage information.
parser = MyArgumentParser(
epilog="Example invocation: python3 hisinone-scraper",
Copy link
Contributor

Choose a reason for hiding this comment

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

That comment is not accurate

formatter_class=argparse.RawDescriptionHelpFormatter)

# Command line arguments.
parser.add_argument(
"--not-headless", dest="not_headless", action="store_true",
help="Run browser in headful mode (default: headless mode)")
parser.add_argument(
"--url", dest="url", type=str,
default="https://qlever.cs.uni-freiburg.de",
help="The URL of the QLever UI (may redirect)")
parser.add_argument(
"--num-retries", dest="num_retries", type=int, default=5,
help="Number of retries for loading a page")
parser.add_argument(
"--log-level", dest="log_level", type=str,
choices=["INFO", "DEBUG", "ERROR"], default="INFO",
help="Log level (INFO, DEBUG, ERROR)")
args = parser.parse_args(sys.argv[1:])

# Set log level and show it.
log.setLevel(eval("logging.%s" % args.log_level))
print()
log.info("Log level is \x1b[1m%s\x1b[0m" % args.log_level)

# Test the QLever UI.
qleverui_tester = QleverUiTester(
not args.not_headless, args.url, args.num_retries)
qleverui_tester.test()
qleverui_tester.done()