Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

  1. Add a helper script to run bash test scripts (*.bats).
  2. Run bats part of CI.

https://issues.apache.org/jira/browse/HDDS-4031

How was this patch tested?

Created failing test at hadoop-hdds/common/src/test/shell/failing.bats:

@test "fails" {
  [[ "123" == "456" ]]
}

@test "err" {
  asdfs
}

and tested:

$ hadoop-ozone/dev-support/checks/bats.sh
...

$ echo $?
1

$ cat target/bats/summary.txt
not ok 1 fails
# (in test file hadoop-hdds/common/src/test/shell/failing.bats, line 24)
#   `@test "fails" {' failed
not ok 2 err
# (in test file hadoop-hdds/common/src/test/shell/failing.bats, line 29)
#   `asdfs' failed with status 127
# /var/folders/32/sr06nr7123s3bdfsmqsm4pqw0000gn/T/bats-run-70119/bats.70145.src: line 29: asdfs: command not found

$ cat target/bats/failures
2

Passing tests (currently only gc_opts.bat):

1..3
ok 1 Setting Hadoop GC parameters: add GC params for server
ok 2 Setting Hadoop GC parameters: disabled for client
ok 3 Setting Hadoop GC parameters: disabled if GC params are customized

https://github.com/adoroszlai/hadoop-ozone/runs/913609741#step:4:4

@adoroszlai adoroszlai self-assigned this Jul 27, 2020
@adoroszlai adoroszlai requested a review from elek July 27, 2020 13:55
Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1 thanks the patch @adoroszlai

fun fact: I started to do the same during the last weekend, but this patch seems to be better.

As usual, I have some random thoughts added (I think it's good to have "water cooler conversations", and as everybody working from remote, PRs seems to be a good place to chat ;-) )

But none of them are blocker. Especially as I have other PRs with more bats tests, I would prefer to merge it as is, now.


rm -f "${REPORT_DIR}/output.log"

find * -path '*/src/test/shell/*' -name '*.bats' -print0 \
Copy link
Member

Choose a reason for hiding this comment

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

discussion-mode:on

This pattern ("fail at the end") is introduced to execute all the tests even if some of them are failing. It's very useful for long and flaky test runs.

But it's somewhat more complex.

Recently I started to think to use a "fail fast" approach. It can simplify the scripts and we can use set -e to be sure we don't ignore any internal errors.

It's not working everywhere, but here it could.

But I am fine with this approach (which is better to create a summary), just sharing my thoughts.

</discussion-mode:on>

bats:
runs-on: ubuntu-18.04
steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

In the future I am planning to improve all the existing tests:

  1. With adding name to all the steps
  2. Use upper case for the first letter (default for github actions if the name is missing).

This is the same as the others, so I think we can merge it as is.

@elek elek merged commit 829b860 into apache:master Jul 29, 2020
@adoroszlai adoroszlai deleted the HDDS-4031 branch July 29, 2020 12:17
@adoroszlai
Copy link
Contributor Author

Thanks @elek for reviewing and committing it.

errose28 added a commit to errose28/ozone that referenced this pull request Jul 31, 2020
* master: (55 commits)
  HDDS-4052. Remove master/slave terminology from Ozone (apache#1281)
  HDDS-4047. OzoneManager met NPE exception while getServiceList (apache#1277)
  HDDS-3990. Test Kubernetes examples with acceptance tests (apache#1223)
  HDDS-4045. Add more ignore rules to the RAT ignore list (apache#1273)
  HDDS-3970. Enabling TestStorageContainerManager with all failures addressed (apache#1257)
  HDDS-4033. Make the acceptance test reports hierarchical (apache#1263)
  HDDS-3423. Enabling TestContainerReplicationEndToEnd and addressing failures (apache#1260)
  HDDS-4027. Suppress ERROR message when SCM attempt to create additional pipelines. (apache#1265)
  HDDS-4024. Avoid while loop too soon when exception happen (apache#1253)
  HDDS-3809. Make number of open containers on a datanode a function of no of volumes reported by it. (apache#1081)
  HDDS-4019. Show the storageDir while need init om or scm (apache#1248)
  HDDS-3511. Fix javadoc comment in OmMetadataManager (apache#1247)
  HDDS-4041. Ozone /conf endpoint triggers kerberos replay error when SPNEGO is enabled. (apache#1267)
  HDDS-4031. Run shell tests in CI (apache#1261)
  HDDS-4038. Eliminate GitHub check warnings (apache#1268)
  HDDS-4011. Update S3 related documentation. (apache#1245)
  HDDS-4030. Remember the selected columns and make the X-axis scrollable in recon datanodes UI (apache#1259)
  HDDS-4032. Run author check without docker (apache#1262)
  HDDS-4026. Dir rename failed when sets 'ozone.om.enable.filesystem.paths' to true (apache#1256)
  HDDS-4017. Acceptance check may run against wrong commit (apache#1249)
  ...
timmylicheng pushed a commit that referenced this pull request Aug 6, 2020
rakeshadr pushed a commit to rakeshadr/hadoop-ozone that referenced this pull request Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants