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

Made CI skip Coveralls if Java files aren't changed #5912

Closed

Conversation

elebitzero
Copy link
Member

@elebitzero elebitzero commented Jun 9, 2023

Part of #5909.

Added job that gets the list of changed files and checks if any are within one of the java directories, then changed=true, otherwise false.

If the Java source is changed, then Java tests are run and the Coveralls report is run.

Changes proposed in this pull request:

  • Made it so Coveralls are skipped on CI if no Java files are altered.

@wetneb
Copy link
Sponsor Member

wetneb commented Jun 9, 2023

Would it not be cleaner to use the native path filtering options for that? It would avoid spinning up a job just to do this computation. It looks like this is only available for the entire workflow, and not an individual job, but perhaps we could just split the pull_request.yml workflow into two? The Windows CI job could go with the two Linux Java CI jobs.

@elebitzero
Copy link
Member Author

I'm not sure, it seems more complicated to split it into multiple workflows, but maybe because I haven't seen it done that way.

I just got it working but I need to try changing a Java source file to see if the Java tests and Coveralls report runs correctly.

I'll do that tomorrow. Let me know by tomorrow if you want me to continue on this.

Thanks,

@wetneb
Copy link
Sponsor Member

wetneb commented Jun 9, 2023

To me it seems simpler for the following reasons:

  • no need for an additional job
  • the Java CI jobs would not get executed at all, instead of booting up to check if they should execute or not (producing spurious green checks in the list of checks run on the PR)
  • we don't need to pull in another github action
  • the pull_request.yml file is getting quite long and separating the backend tests from the e2e ones seem like a good opportunity to make the size more manageable

@elebitzero
Copy link
Member Author

If we change windows_server_test to always skip the Java tests, then you could move the check_java_src_changes into the linux_server_test and avoid creating another job. I had to have as a separate job because both the Windows and Linux tests were using the result.

But if you are saying to skip all server tests if no Java code is changed, and to rely on the e2e tests, I wasn't thinking that is possible but maybe it is ok.

I thought there may be cases where you make some change that isn't in the Java source directory, the server tests should still be run to make sure nothing got broken, like if the refine or refine.bat script is modified.

@wetneb
Copy link
Sponsor Member

wetneb commented Jun 9, 2023

For sure the list of files I mentioned in #5912 is not exhaustive - it does make sense to add ./refine and refine.bat to it for instance. Worst case scenario, if we forget to list a file that the backend depends on, the snapshot release CI will catch it directly after merging and we can fix the list of files and directory afterwards.

Once we detect that no changes were made to the files, we should not have to run any mvn command, basically. Compiling is not needed either (and it will be done by the Cypress jobs anyway).

@elebitzero elebitzero closed this Jun 9, 2023
@elebitzero elebitzero deleted the non-java-coverage-skip branch July 13, 2023 17:39
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.

None yet

2 participants