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

Test clean task in CI #896

Merged
merged 17 commits into from
Sep 5, 2024
Merged

Test clean task in CI #896

merged 17 commits into from
Sep 5, 2024

Conversation

Ao-senXiong
Copy link
Member

@Ao-senXiong Ao-senXiong commented Aug 30, 2024

Fixes #413

@wmdietl
Copy link
Member

wmdietl commented Aug 30, 2024

The problem on Windows is the ../../../gradlew clean which appears in one of the Makefiles.

@Ao-senXiong
Copy link
Member Author

The problem on Windows is the ../../../gradlew clean which appears in one of the Makefiles.

Thanks!

@Ao-senXiong
Copy link
Member Author

The problem on Windows is the ../../../gradlew clean which appears in one of the Makefiles.

Hi @wmdietl, do you have any idea how to fix the problem? I did 0b1e15c this for windows but it looks like it is taking forever to run test for windows and fails eventually.

@@ -17,3 +17,6 @@ source "$SCRIPTDIR"/clone-related.sh
# Moved example-tests out of all tests because it fails in
# the release script because the newest maven artifacts are not published yet.
./gradlew :checker:exampleTests -x javadoc -x allJavadoc --console=plain --warning-mode=all

Copy link
Member

Choose a reason for hiding this comment

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

Is there any benefit to also test this here? Isn't it enough to test in the junit script?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think :checker:examplesTests are tests run makefiles

task exampleTests(type: Exec, dependsOn: assembleForJavac, group: 'Verification') {

Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate a bit what your point is?
We already test running ./gradlew clean in junit above. Do you want to clean at the end of every test? Or why is :checker:examplesTests relevant for whether we call clean here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think Junit test doesn't contain makefile tests such as exampleTests, is that correct? Because exampleTests are the tests for makefiles and ./gradlew clean will also run make clean for makefile tests. I just want to make it works after we actual run makefile tests in CI.

Copy link
Member

Choose a reason for hiding this comment

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

There are no tests that actually ensure that clean cleaned up everything - so you won't know whether this call to clean actually removed all files that were generated from exampleTests.
So what do you learn from calling clean here?

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, I will clean up them. Thanks!

@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Sep 3, 2024
@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Sep 4, 2024
@wmdietl wmdietl changed the title Use simpler script and add clean task to CI Test clean task in CI Sep 4, 2024
@Ao-senXiong Ao-senXiong requested a review from wmdietl September 4, 2024 20:55
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Sep 4, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks!

@wmdietl wmdietl merged commit 8727b59 into eisop:master Sep 5, 2024
74 checks passed
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.

Improve cleaning the project
2 participants