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

Fixes #180 - Migrate 'PerformanceTest' to be a real integration test #196

Merged
merged 5 commits into from
Aug 9, 2018

Conversation

mawiesne
Copy link
Contributor

@mawiesne mawiesne commented Aug 3, 2018

  • Adds two different site-specific Maven profiles: mysql-ukp and mysql-hhn
  • Adds Maven configuration for IT-setup (via maven-failsafe-plugin)
  • Introduces the new src branch src/it/java in wikipedia.api module
  • Moves and refactors existing code of PerformanceTest and PerformanceTestPageIterator to be executable in an IT environment
  • Introduces PerformanceIT as a true integration test
  • Adds MySQL/C3P0 dependencies in test scope to be capable of running against MySQL as if in a production setup

- Adds two different site-specific Maven profiles: `mysql-ukp` and `mysql-hhn`
- Adds Maven configuration for IT-setup (via maven-failsafe-plugin)
- Introduces the new src branch `src/it/java` in wikipedia.api module
- Moves and refactors existing code of `PerformanceTest` and `PerformanceTestPageIterator` to be executable in an IT environment
- Introduces `PerformanceIT` as a true integration test
- Adds MySQL/C3P0 dependencies in test scope to be capable of running against MySQL as if in a production setup
@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 3, 2018

@reckart Please review this and check the Maven profile mysql-ukp for it's values. If possible, test this profile in Darmstadt. If changes are needed please update the related pom.xml accordingly, so this can work in future CI/PR runs.

FYI:
I succeeded with the mysql-hhn active in our system environment (i.e.: DB/CI servers).

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 3, 2018

It seems PerformanceIT is run even though the profile integration-test should not be active by default (?!?!)

Jenkins output:

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running de.tudarmstadt.ukp.wikipedia.api.PerformanceIT
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.464 s
...

Atm, I don't get why this runs within the CI server. 🤔

EDIT:
I fixed this problem with 9bb1e7e, Jenkins should be happy again.
Note: A mvn command line (locally or on CI servers) should now add -Pjacoco -Pintegration-test -Pmysql-ukp, or HHN specific ... -Pmysql-hhn.

- Corrects integration-test profile setup. By default ITs are switched off now, as regular devs need a build that is passing
- A working mvn commandline (locally or on CI servers) should now add `-Pjacoco -Pintegration-test -Pmysql-ukp`
@mawiesne mawiesne changed the title #180 - Migrate 'PerformanceTest' to be a real integration test Fixes #180 - Migrate 'PerformanceTest' to be a real integration test Aug 3, 2018
as they might not have access to one of the two institution specific
MySQL resource profiles.
-->
<skip.integration.tests>true</skip.integration.tests>
Copy link
Member

Choose a reason for hiding this comment

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

I suppose you could just set the skipITs property directly. Cf. https://maven.apache.org/surefire/maven-failsafe-plugin/integration-test-mojo.html#skipITs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reckart 9bb1e7e fixed it, so it works fine for devs outside UKP/HHN. People or CI-servers inside UKP/HHN can now choose to run ITs. See my updated comment above.

<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<scope>test</scope>
<scope>runtime</scope>
Copy link
Member

Choose a reason for hiding this comment

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

runtime dependencies are transitive. Logging backend dependencies should not be included as transitive dependencies by libraries. Cf. https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope

Copy link
Contributor Author

@mawiesne mawiesne Aug 3, 2018

Choose a reason for hiding this comment

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

fixed with b8ce4b4

int pageCycles = Integer.parseInt(configuration.getProperty("performance.cycles.page"));
pt = new PerformanceTest(wiki, maxiCycles, pageCycles);
} catch (Exception e) {
fail("Wikipedia could not be initialized: "+e.getLocalizedMessage());
Copy link
Member

Choose a reason for hiding this comment

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

Just throw the exception. This "eats" the stack trace.

Copy link
Contributor Author

@mawiesne mawiesne Aug 3, 2018

Choose a reason for hiding this comment

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

fixed with b8ce4b4

@ukp-svc-jenkins
Copy link

24% (+0.61%) vs master 23%

@reckart
Copy link
Member

reckart commented Aug 3, 2018

@mawiesne Eventually, the UKP-internal DB server will go away. So I think it probably doesn't make too much sense to test against it. If I were to test against it, then IMHO it would be sensible to test against it in addition to testing against HSQL and not testing only against the MySQL server. But I believe currently I could not activate both profiles, right?

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 3, 2018

@reckart You can use both profiles the same time, try -Pintegration-test -Pmysql-ukp. Surefire will always execute existing JUnit-like tests, whereas Failsafe will - in addtion - execute PerformanceIT, given of course that it can access your MySQL instance.

The output should look like this (output here at HHN system environment):

2018-08-03 14:02:54 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:02:54 DEBUG PerformanceTest:188 - Used Buffer Size: 1
2018-08-03 14:02:54 DEBUG PerformanceTest:189 - Time            : 2901ms
2018-08-03 14:02:54 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:02:54 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:02:54 DEBUG PerformanceTest:188 - Used Buffer Size: 10
2018-08-03 14:02:54 DEBUG PerformanceTest:189 - Time            : 611ms
2018-08-03 14:02:54 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:02:55 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:02:55 DEBUG PerformanceTest:188 - Used Buffer Size: 50
2018-08-03 14:02:55 DEBUG PerformanceTest:189 - Time            : 421ms
2018-08-03 14:02:55 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:03:30 DEBUG PerformanceTest:135 - -----------------
2018-08-03 14:03:30 DEBUG PerformanceTest:136 - average throughput: 0.0034021760764861237 pages/ms
2018-08-03 14:03:30 DEBUG PerformanceTest:137 - average throughput: 3.4021760764861235 pages/s
2018-08-03 14:03:30 DEBUG PerformanceTest:137 - -----------------
2018-08-03 14:03:48 DEBUG PerformanceTest:135 - -----------------
2018-08-03 14:03:48 DEBUG PerformanceTest:136 - average throughput: 0.008365117257840613 pages/ms
2018-08-03 14:03:48 DEBUG PerformanceTest:137 - average throughput: 8.365117257840614 pages/s
2018-08-03 14:03:48 DEBUG PerformanceTest:137 - -----------------
2018-08-03 14:03:48 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:03:48 DEBUG PerformanceTest:188 - Used Buffer Size: 100
2018-08-03 14:03:48 DEBUG PerformanceTest:189 - Time            : 406ms
2018-08-03 14:03:48 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:03:48 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:03:48 DEBUG PerformanceTest:188 - Used Buffer Size: 200
2018-08-03 14:03:48 DEBUG PerformanceTest:189 - Time            : 394ms
2018-08-03 14:03:48 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:03:49 DEBUG PerformanceTest:187 - RetrievedPages  : 1000
2018-08-03 14:03:49 DEBUG PerformanceTest:188 - Used Buffer Size: 500
2018-08-03 14:03:49 DEBUG PerformanceTest:189 - Time            : 383ms
2018-08-03 14:03:49 DEBUG PerformanceTest:190 - ------------------------------
2018-08-03 14:03:52 DEBUG PerformanceTest:102 - -----------------
2018-08-03 14:03:52 DEBUG PerformanceTest:103 - average throughput: 0.037308212272344266 pages/ms
2018-08-03 14:03:52 DEBUG PerformanceTest:104 - average throughput: 37.308212272344264 pages/s
2018-08-03 14:03:52 DEBUG PerformanceTest:105 - -----------------
2018-08-03 14:03:52 DEBUG PerformanceTest:102 - -----------------
2018-08-03 14:03:52 DEBUG PerformanceTest:103 - average throughput: 0.45482121518521923 pages/ms
2018-08-03 14:03:52 DEBUG PerformanceTest:104 - average throughput: 454.8212151852192 pages/s
2018-08-03 14:03:52 DEBUG PerformanceTest:105 - -----------------

@reckart
Copy link
Member

reckart commented Aug 3, 2018

I have added the profiles integration-test and mysql-ukp to the PR build job as well as to the post-merge build job. So the next build running on this PR should also try running the integration tests. Let's see what happens ;)

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 3, 2018

@reckart see my TODO in pom.xml of wikipedia.api module:

<!-- TODO @reckart: check if this works and/or modify it so this profile works at UKP Lab -->

If the values are fine, then remove this line from the pom.xml or modify it and push it. Once correctly setup, we have two institutions that can verify performance aspects or if mysql setup is working. Here in HHN, the mysql setup will not vanish too soon, i.e. at least one institution can conduct this kind of verification.

Any way, both profiles (integration-test, mysql-*) are optional from an external dev's perspective, e.g. @tgalery

@ukp-svc-jenkins
Copy link

24% (+0.61%) vs master 23%

- Removes a TODO to trigger a CI build as suggested by @reckart in PR #196
@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 6, 2018

@reckart It seems we need other credentials or the DBA setting some permissions.

Access denied for user 'wikiapi'@'roberto.ukp.informatik.tu-darmstadt.de' (using password: YES)

@reckart
Copy link
Member

reckart commented Aug 6, 2018

@mawiesne the hostname is wrong. It should be "bender.ukp.informatik.tu-darmstadt.de".

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 6, 2018

@reckart Look at the value in the pom.xml.

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 6, 2018

@reckart I understood the problem: Roberto is Jenkins and if the IT code is run from there it connects from there. So you have to GRANT access for that user at this machine. That‘s what the error message is telling us: wikiapi as username from roberto... as machine/host name wants to connect to bender... but isn‘t allowed to atm.

@reckart
Copy link
Member

reckart commented Aug 8, 2018

The problem is probable the username/password used. Could it be that you mixed up the credentials for mysql-hnn and mysql-ukp? Username and pw for UKP should be student.

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 9, 2018

I merely took the credentials from the existing code. See original PerformanceTest in the master branch (or diff editor of this PR).

@reckart
Copy link
Member

reckart commented Aug 9, 2018

@mawiesne Then I guess the test wouldn't have run because the user wikiapi doesn't actually seem to exist on our database server.

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 9, 2018

The test seemed to be broke before then. So @reckart, please change the values to student/student then, so a build is triggered.

Background: I can’t do this atm, as I‘m on vacation and without an IDE for the next 2 weeks.

- Changed credentials for UKP database server used during test
@reckart
Copy link
Member

reckart commented Aug 9, 2018

@mawiesne ok, I made the change via the web interface of GitHub :) Let's see how the build goes.

@ukp-svc-jenkins
Copy link

24% (+0.61%) vs master 23%

@mawiesne
Copy link
Contributor Author

mawiesne commented Aug 9, 2018

Worked, nice. (:
So: merge the PR then?

@reckart reckart merged commit 1eb69d3 into master Aug 9, 2018
@reckart reckart deleted the refactoring/180-migrate-PerformanceTest-to-IT branch August 9, 2018 17:18
@reckart
Copy link
Member

reckart commented Aug 9, 2018

@mawiesne Merged 👍

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

Successfully merging this pull request may close these issues.

3 participants