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.

Sign up for GitHub

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

Jump to bottom

Migrate systemd packaging tests from bats to java #39954

Merged

pgomulka merged 18 commits into elastic:master from pgomulka:feature/packaging-tests

Mar 28, 2019

Contributor

pgomulka commented Mar 12, 2019 •

edited

Loading

Migrating systemd bats tests from bats to java dsl. This also covers partially the sysv, but more must be added

relates #32143

pgomulka added >test :Delivery/Packaging WIP labels

pgomulka self-assigned this

Collaborator

elasticmachine commented Mar 12, 2019

Pinging @elastic/es-core-infra

andyb-elastic mentioned this pull request

java packaging tests #32143

Closed

24 tasks


          initial work

48bfbaf

pgomulka force-pushed the feature/packaging-tests branch from 80fb18a to 48bfbaf Compare

March 12, 2019 13:29

pgomulka changed the title ~~WIP migrate systemd and sysv from bats to java~~ WIP migrate systemd and sysv packaging tests from bats to java

pgomulka added 11 commits

March 13, 2019 18:29


          further testing

6b6b2fe


          migrated

480fc6c


          fix build

2eac5c4


          test

3242bbf


          passing test with systemd

af6a1fe


          Merge branch 'master' into feature/packaging-tests

9d2388b


          fix compilation

d2af454


          failing new test

3d7c0a7


          all tests passing systemd

61ef58f


          testcases in package test

1fc5134


          passing all tests for systemd

18c0210

Contributor Author

pgomulka commented Mar 21, 2019

This is now ready for a review. The systemd part has been migrated.

pgomulka removed the WIP label

pgomulka changed the title ~~WIP migrate systemd and sysv packaging tests from bats to java~~ WIP migrate systemd packaging tests from bats to java

pgomulka changed the title ~~WIP migrate systemd packaging tests from bats to java~~ Migrate systemd packaging tests from bats to java


          limits are applied for systemd only

f2ec8ac

Contributor Author

pgomulka commented Mar 22, 2019

@elasticmachine run elasticsearch-ci/packaging

alpar-t reviewed

View reviewed changes

Contributor

alpar-t left a comment

Thanks for taking this on @pgomulka I love to see the bats tests go !
I left a few comments and questions.

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java

+                      stopElasticsearch(newShell());
+                  }
+                  // TEST CASES FOR SYSTEMD ONLY

Contributor

alpar-t Mar 22, 2019

In the interest of keeping this class from growing, could we have a separate class fro the systemd tests ?

Contributor Author

pgomulka Mar 22, 2019

There is 8 subclasses of this testcase now, if we separate systemd from sysv we will endup with 16. Most of the time we share the logic for testing between systemd and sysv. I think this is ok for now, but definitely we should consider restructuring the hierarchy once this grow too big.

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java Outdated

+                      installation = install(distribution());
+                      assertInstalled(distribution());
+                      Shell sh = newShell();

Contributor

alpar-t Mar 22, 2019

would it make sense to have this as a field set up in a @Before ?

Contributor Author

pgomulka Mar 22, 2019

good idea, this is repeated all over the testcases

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java Outdated



		public void test72TestRuntimeDirectory() throws IOException {
		cleanup();

Contributor

alpar-t Mar 22, 2019

could this be done in @After or @Before if you wish to avoid the repetitive calls and make sure we always start clean ? if we have tests that break with this now, I think that's something we need to fix as it makes the test hard to read if different tests depend on each-other even when run in sequence.

Contributor Author

pgomulka Mar 22, 2019

right, I think not all tests are leaving the environment dirty. Might be better to keep cleanup in the test that is making the changes to environment in try-finally block

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java Outdated

+                  public void test72TestRuntimeDirectory() throws IOException {
+                      cleanup();
+                      installation = install(distribution());
+                      FileUtils.rm(installation.pidDir);

Contributor

alpar-t Mar 22, 2019

nit: static imports of the methods in FileUtils would make this slightly more readbale

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/PackageTestCase.java Outdated

+                      installation = install(distribution());
+                      startElasticsearch(newShell());
+                      // it can be gc.log or gc.log.0.current
+                      assertThat(installation.logs, FileUtils.fileWithRegexExist("gc.log*"));

Contributor

alpar-t Mar 22, 2019

I think technically that's a glob not a regexp.

Contributor Author

pgomulka Mar 22, 2019

right, good spot

qa/vagrant/src/main/java/org/elasticsearch/packaging/util/Packages.java Outdated


		waitForElasticsearch();

		// assertStatuses();

Contributor

alpar-t Mar 22, 2019

is this intentionally commented out ?

Contributor Author

pgomulka Mar 22, 2019

I had problems with running status after restart. Will have a second look but for time being will remove it.

qa/vagrant/src/test/resources/packaging/tests/60_systemd.bats

-                  # Rather than restart the VM we just ask systemd if it plans on starting
-                  # elasticsearch on restart. Not as strong as a restart but much much
-                  # faster.
-                  run systemctl is-enabled elasticsearch.service

Contributor

alpar-t Mar 22, 2019

Did we miss porting this check ?

Contributor Author

pgomulka Mar 26, 2019

yes I missed this one. added

qa/vagrant/src/test/resources/packaging/tests/60_systemd.bats

-                  systemctl start elasticsearch.service
-                  wait_for_elasticsearch_status
-                  assert_file_exist "/var/run/elasticsearch/elasticsearch.pid"
-                  assert_file_exist "/var/log/elasticsearch/elasticsearch_server.json"

Contributor

alpar-t Mar 22, 2019

Do we check this in the java version ?

Contributor Author

pgomulka Mar 26, 2019 •

edited

Loading

good spot, it was not checked at all for packages tests

qa/vagrant/src/test/resources/packaging/tests/60_systemd.bats

-                  result="$(journalctl _SYSTEMD_UNIT=elasticsearch.service --since "$since" --output cat | wc -l)"
-                  [ "$result" -eq "0" ] || {
-                          echo "Expected no entries in journalctl for the Elasticsearch service but found:"
-                          journalctl _SYSTEMD_UNIT=elasticsearch.service --since "$since"

Contributor

alpar-t Mar 22, 2019

I think we are also missing this one.

Contributor Author

pgomulka Mar 27, 2019

Added. I wrongly assumed that since start is already migrated that I didn't have to look into this step :)

qa/vagrant/src/test/resources/packaging/tests/60_systemd.bats

-              @test "[SYSTEMD] test runtime directory" {
-                  clean_before_test
-                  install_package
-                  sudo rm -rf /var/run/elasticsearch

Contributor

alpar-t Mar 22, 2019

might be missing this one too ?

Contributor Author

pgomulka Mar 26, 2019

should be now in test72TestRuntimeDirectory

Conky5 commented Mar 22, 2019

The last packaging build was impacted by https://github.com/elastic/infra/issues/10287.

@elasticmachine run elasticsearch-ci/packaging

pgomulka added 2 commits

March 25, 2019 11:38


          code review followup

882aa27


          Merge branch 'master' into feature/packaging-tests

5cd1075

pgomulka added 3 commits

March 25, 2019 16:40


          is enabled return false

70f3e92


          missing assertions

e09f1e2


          json log name

aa9a365

pgomulka requested a review from alpar-t

March 27, 2019 13:28

alpar-t approved these changes

View reviewed changes

Contributor

alpar-t left a comment

LGTM

Contributor

alpar-t commented Mar 28, 2019

@pgomulka the PR doesn't have any version labels. I do think this should be back-ported to 7.x

pgomulka added v8.0.0 v7.2.0 labels

pgomulka merged commit 5ceef9e into elastic:master

pgomulka added the backport pending label

jasontedor added a commit to rjernst/elasticsearch that referenced this pull request


          Merge branch 'master' into pr/40576

828ac34

* master: (25 commits)
  [DOCS] Correct keystore commands for Email and Jira actions in Watcher (elastic#40417)
  [DOCS] Document common settings for snapshot repository plugins (elastic#40475)
  Remove with(out)-system-key tests (elastic#40547)
  Geo Point parse error fix (elastic#40447)
  Handle null retention leases in WaitForNoFollowersStep (elastic#40477)
  [DOCS] Adds anchors for ruby client (elastic#39867)
  Mute DataFrameAuditorIT#testAuditorWritesAudits
  Disable integTest when Docker is not available (elastic#40585)
  Add randomScore function in script_score query (elastic#40186)
  Test fixtures krb5 (elastic#40297)
  Correct ILM metadata minimum compatibility version (elastic#40569)
  SQL: Centralize SQL test dependencies version handling (elastic#40551)
  Mute testTracerLog
  Mute testHttpInput
  Include functions' aliases in the list of functions (elastic#40584)
  Optimise rejection of out-of-range `long` values (elastic#40325)
  Add docs for cluster.remote.*.proxy setting (elastic#40281)
  Migrate systemd packaging tests from bats to java (elastic#39954)
  Move top-level pipeline aggs out of QuerySearchResult (elastic#40319)
  Use openjdk 12 in packer cache script (elastic#40498)
  ...

pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request


          Migrate systemd packaging tests from bats to java backport(elastic#39954

0d6ecce

)

Migrating systemd bats tests from bats to java dsl. This also covers
partially the sysv, but more must be added

relates elastic#32143

pgomulka mentioned this pull request

Migrate systemd packaging tests from bats to java backport(#39954) #40763

Merged

pgomulka added a commit that referenced this pull request


          Migrate systemd packaging tests from bats to java backport(#39954) (#…

a6faf85

…40763)

Migrating systemd bats tests from bats to java dsl. This also covers
partially the sysv, but more must be added

relates #32143
backport #39954

pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request


          Migrate systemd packaging tests from bats to java backport(elastic#39954

6f89742

)

Migrating systemd bats tests from bats to java dsl. This also covers
partially the sysv, but more must be added

relates elastic#32143

pgomulka mentioned this pull request

Bat scripts to work with JAVA_HOME with parentheses Backports #40770

Merged

pgomulka added a commit that referenced this pull request


           Bat scripts to work with JAVA_HOME with parentheses Backports (#40770)

e15e337

backports :
Migrate systemd packaging tests from bats to java (#39954)
Bat scripts to work with JAVA_HOME with parentheses (#39712)
Link to SYSTEM_JAVA_HOME on windows (#40806)

jakelandis added v7.3.0 and removed v7.2.0 labels

Contributor

jpountz commented Jul 5, 2019

@pgomulka I'm assuming there is nothing left to backport and removed the backport pending label.

jpountz removed the backport pending label

mark-vieira added the Team:Delivery label

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels

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

Labels

:Delivery/Packaging Team:Delivery >test v7.3.0 v8.0.0-alpha1