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

Switch to GitHub Actions CI #1163

Closed
wants to merge 43 commits into from
Closed

Switch to GitHub Actions CI #1163

wants to merge 43 commits into from

Conversation

osma
Copy link
Member

@osma osma commented May 20, 2021

Initial draft PR trying to implement GitHub Actions CI instead of Travis CI.

Fixes #1147

TODO items:

  • Use the same PHPUnit version as declared in composer.json
  • Cache the Composer dependencies
  • Install Fuseki and load the test vocabularies into it
  • Cache the Fuseki installation
  • Run the tests on all supported PHP versions in parallel
  • Report code coverage to Code Climate
  • Report code coverage to Codecov
  • Report passing/failing tests to Slack

@osma osma added this to the 2.11 milestone May 20, 2021
@osma osma self-assigned this May 20, 2021
@osma
Copy link
Member Author

osma commented May 20, 2021

I'm hitting some obstacles here...

I'd like to keep the test suite as it is, so that it can be easily run on a developer machine, without involving e.g. Docker containers. So in this case, Fuseki needs to be running and initialized with the test data, which is normally done using the tests/init_fuseki.sh script. Then Fuseki can be accessed as http://localhost:13030/ (the default port 3030 is not used because that could interfer with another running instance of Fuseki). This works since the testconfig.ttl file specifies http://localhost:13030/ as the SPARQL endpoint to use.

Under GitHub Actions, the normal way to run databases needed for unit tests is to set them up as service containers. So e.g. the stain/jena-fuseki image would be downloaded from Docker Hub and running in its own container. This seems pretty convenient once it's been set up, there's no need for the hassle of trying to download the correct Fuseki distribution archive from apache.org.

Now there are two ways of running GitHub Actions:

  1. Directly on the runner machine (without a container). This means that the test scripts etc. will be run in a fairly normal Ubuntu Linux environment. If Fuseki is running as a service container, it can be mapped to localhost:13030 using a ports rule.
  2. Within its own container. In this case, a Docker internal network is automatically set up. Fuseki can be accessed using a name such as fuseki:3030 - the hostname is taken from the service name.

OTOH, it would be nice to run the unit tests in parallel on multiple PHP versions. The normal Ubuntu 20.04 base image provides PHP 7.4 and 8.0, but no other versions (e.g. 7.2 or 7.3). There is a set of Actions modules(?) called php-actions which makes it possible to use other PHP versions - but in this case, the PHP environment is running in a container.

So this leaves a choice:

  1. Run the tests directly on the runner machine, using the Ubuntu base image. Tests can be only run on PHP 7.4 (and possibly 8.0). We can access Fuseki as http://localhost:13030/ so there's no need to change testconfig.ttl, it will keep working as it is also on developer machines.
  2. Use the php-actions method for running tests under multiple PHP versions within containers. In this setting it's not possible to access Fuseki as http://localhost:13030/, it would have to be http://fuseki:3030/ or something similar. We have to change testconfig.ttl or make an alternative version of it.

There might be a way of running older/alternative PHP versions also in scenario 1, since many versions of PHP are available in Ondřej Surý's PPA and it's possible to install custom deb packages in the runner environment. But I'm a bit hesitant to go that way since php-actions seems to provide all the infrastructure necessary for running multiple PHP versions, in a nice package.

@osma
Copy link
Member Author

osma commented May 20, 2021

In fact the localhost:3030 address is used not only in testconfig.ttl but also many of the tests...

$ grep -c localhost:13030 *.php *.ttl|grep -v ':0'
GenericSparqlTest.php:21
GlobalConfigTest.php:1
JenaTextSparqlTest.php:5
ModelTest.php:1
VocabularyTest.php:1
jenatestconfig.ttl:21
testconfig-fordefaults.ttl:1
testconfig.ttl:32

So the address is pretty heavily hardcoded in to the test suite. In the .php files we could probably read it from, say, an environment variable, but for the .ttl files there is no simple templating or variable expansion mechanism which would allow using two different addresses in different settings...

Any ideas @kinow?

@osma
Copy link
Member Author

osma commented May 20, 2021

I think I figured out a good solution...we should make GlobalConfig (and Vocabulary) check if there's an environment variable (FUSEKI_ADDRESS or something along those lines) and fall back to that if no sparqlEndpoint setting is given in the configuration file. I'll try make a PR for that. Then the test suite can simply omit the current hardcoded URL and set it through environment variables instead.

@kinow
Copy link
Collaborator

kinow commented May 21, 2021

I think I figured out a good solution...we should make GlobalConfig (and Vocabulary) check if there's an environment variable (FUSEKI_ADDRESS or something along those lines) and fall back to that if no sparqlEndpoint setting is given in the configuration file. I'll try make a PR for that. Then the test suite can simply omit the current hardcoded URL and set it through environment variables instead.

Sounds like a good solution. I prefer the approach that tests multiple PHP versions too, while also isolating the testing environment with a container.

If the env var approach becomes too hard to implement now, maybe we could simply have a port-forwarder listening for requests to localhost:13030 and forwarding them to fuseki:3030 in the GH actions env? I guess that should work too. I normally use an old Java program I have for that, or netcat or nc.

Not elegant but only in case you'd prefer to leave the env var for later if it becomes too complicated 👍

@osma
Copy link
Member Author

osma commented May 24, 2021

If the env var approach becomes too hard to implement now, maybe we could simply have a port-forwarder listening for requests to localhost:13030 and forwarding them to fuseki:3030 in the GH actions env? I guess that should work too. I normally use an old Java program I have for that, or netcat or nc.

I thought about this too... But I don't see how it would help in the scenario where it would matter. If we want to easily run tests on multiple PHP versions in parallel, we need to use the php-actions method where PHPUnit is run in its own container built using php-build based on the official PHP container - or at least that's how I understood it, I haven't studied it very deeply. We'd somehow have to smuggle in netcat or similar inside that container. It might be possible but it seems likely that it would require forking php-build. That doesn't sound like an easy approach to me...

@osma
Copy link
Member Author

osma commented May 25, 2021

Now that it's possible to specify the Fuseki endpoint using an environment variable (#1166) I tried again setting this up via php-actions/phpunit. However, I hit new issues: the PHPUnit container is sitting on the default bridge network, so it cannot access the Fuseki service container. I opened an issue in php-actions/phpunit explaining the problem and suggesting potential fixes.

@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #1163 (33408f7) into master (4b538db) will decrease coverage by 0.08%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1163      +/-   ##
============================================
- Coverage     67.97%   67.89%   -0.09%     
- Complexity     1584     1587       +3     
============================================
  Files            32       32              
  Lines          3888     3884       -4     
============================================
- Hits           2643     2637       -6     
- Misses         1245     1247       +2     
Impacted Files Coverage Δ
model/Vocabulary.php 87.30% <66.66%> (-0.56%) ⬇️
model/GlobalConfig.php 88.88% <100.00%> (-0.74%) ⬇️
model/resolver/WDQSResource.php 94.11% <0.00%> (-0.33%) ⬇️
model/sparql/JenaTextSparql.php 91.83% <0.00%> (-0.17%) ⬇️
model/sparql/GenericSparql.php 92.03% <0.00%> (-0.09%) ⬇️
model/BaseConfig.php 71.42% <0.00%> (+4.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b538db...33408f7. Read the comment docs.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 19 Code Smells

No Coverage information No Coverage information
8.5% 8.5% Duplication

@osma
Copy link
Member Author

osma commented May 26, 2021

This is now basically working but the git history is very messy. I'm closing this and opening a new PR instead, with cleaner history.

@osma osma closed this May 26, 2021
@osma osma mentioned this pull request May 26, 2021
8 tasks
@osma osma deleted the issue1147-github-actions-ci branch January 20, 2022 13:35
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.

Switch from Travis CI to GitHub Actions
2 participants