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

[Github Flow] pom.xml change is considered a documentation change #8362

Closed
lhotari opened this issue Oct 23, 2020 · 5 comments
Closed

[Github Flow] pom.xml change is considered a documentation change #8362

lhotari opened this issue Oct 23, 2020 · 5 comments
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@lhotari
Copy link
Member

lhotari commented Oct 23, 2020

The current Github Flow's "Check if this pull request only changes documentation" step (id=docs) considers a pom.xml change as a documentation change. When there's a commit that doesn't contain other changes, all tests will be skipped.

Expected behavior
pom.xml shouldn't be considered as a documentation change and when any pom.xml file is change, it should run tests.

Additional context
#8351 passed the checks without running any tests. Example: https://github.com/apache/pulsar/runs/1295771802
There's #8361 to fix the broken state of CI.
See #7955 which introduced the skipping of tests when changes are only in documents. One possible solution is to revert those changes.

@lhotari
Copy link
Member Author

lhotari commented Oct 23, 2020

It seems that this script is eventually used in the checks:
https://github.com/apache/pulsar-test-infra/blob/master/diff-only/entrypoint.sh
it's missing a feature to specify exception patterns, I guess. If the root pom.xml file changes, it's not a doc change.

sijie pushed a commit that referenced this issue Oct 23, 2020
- there are grpc/protobuf version compatibility issues

- tests weren't run because of #8362

This reverts commit 647d3c2.
@lhotari
Copy link
Member Author

lhotari commented Oct 24, 2020

To debug the issue I stored the logs from the pipeline to a gist: https://gist.github.com/lhotari/a4b7aa97d240b6914cfbb3935776c2ea#file-ci_bug_logs-txt-L675

@lhotari
Copy link
Member Author

lhotari commented Oct 24, 2020

It seems that there's a bug in https://github.com/apache/pulsar-test-infra/blob/master/diff-only/entrypoint.sh .
it doesn't seem to handle the case when a commit includes changes only in the root directory
some log rows:

2020-10-23T01:26:37.4699758Z HEAD
2020-10-23T01:26:37.7372668Z CHANGED_DIRS are : 
2020-10-23T01:26:37.7376810Z Changes  only in site2 deployment .asf.yaml .ci ct.yaml, setting 'changed_only' to 'yes'
2020-10-23T01:26:38.9524837Z Post job cleanup.

@lhotari
Copy link
Member Author

lhotari commented Oct 24, 2020

I pushed a fix to diff-only apache/pulsar-test-infra#11

@jiazhai
Copy link
Member

jiazhai commented Oct 26, 2020

Thanks @lhotari for the help

@jiazhai jiazhai closed this as completed Oct 26, 2020
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this issue Nov 13, 2020
- there are grpc/protobuf version compatibility issues

- tests weren't run because of apache#8362

This reverts commit 647d3c2.
flowchartsman pushed a commit to flowchartsman/pulsar that referenced this issue Nov 17, 2020
- there are grpc/protobuf version compatibility issues

- tests weren't run because of apache#8362

This reverts commit 647d3c2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

2 participants