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

Migrate diktat smoke tests to SAVE-cli mechanism #1388

Merged
merged 58 commits into from
Jul 15, 2022

Conversation

Cheshiriks
Copy link
Member

What's done:

### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
@Cheshiriks Cheshiriks marked this pull request as draft June 21, 2022 16:13
@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #1388 (23ec13e) into master (7582190) will decrease coverage by 0.61%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1388      +/-   ##
============================================
- Coverage     83.23%   82.62%   -0.62%     
+ Complexity     2562     2542      -20     
============================================
  Files           107      104       -3     
  Lines          7619     7472     -147     
  Branches       2103     2086      -17     
============================================
- Hits           6342     6174     -168     
- Misses          388      435      +47     
+ Partials        889      863      -26     
Flag Coverage Δ
unittests 82.62% <ø> (-0.62%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../kotlin/org/cqfn/diktat/plugin/maven/DiktatMojo.kt 9.09% <0.00%> (-81.82%) ⬇️
...lin/org/cqfn/diktat/plugin/maven/DiktatBaseMojo.kt 9.57% <0.00%> (-43.62%) ⬇️
...n/org/cqfn/diktat/plugin/gradle/DiktatExtension.kt
...rg/cqfn/diktat/plugin/gradle/DiktatGradlePlugin.kt
...qfn/diktat/plugin/gradle/DiktatJavaExecTaskBase.kt

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7582190...23ec13e. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2022

JUnit Tests (Windows, EnricoMi/publish-unit-test-result-action@v1)

1 300 tests   1 285 ✔️  2m 11s ⏱️
   159 suites       15 💤
   159 files           0

Results for commit 23ec13e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2022

JUnit Tests (macOS, EnricoMi/publish-unit-test-result-action@v1)

1 286 tests   1 270 ✔️  49s ⏱️
   158 suites       16 💤
   158 files           0

Results for commit 23ec13e.

♻️ This comment has been updated with latest results.

@nulls nulls self-requested a review June 22, 2022 14:31
@orchestr7
Copy link
Member

Unit test for downloading of ktlint and putting diktat to a proper directory?

Copy link
Member

@orchestr7 orchestr7 left a comment

Choose a reason for hiding this comment

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

we need a unit test here

### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
@Cheshiriks Cheshiriks marked this pull request as ready for review June 28, 2022 09:39
Copy link
Member

@petertrr petertrr left a comment

Choose a reason for hiding this comment

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

@Cheshiriks please remove binaries from git, instead please either use an HTTP client in Java to download them before test or use maven plugin for downloading. Binaries should be placed in target directory so that they won't show up in Git and could be cleaned up by maven clean

@Cheshiriks
Copy link
Member Author

@Cheshiriks please remove binaries from git, instead please either use an HTTP client in Java to download them before test or use maven plugin for downloading. Binaries should be placed in target directory so that they won't show up in Git and could be cleaned up by maven clean

Haven't we decided that for now we will just put the Save in the project, and later we will do the download of the file?

@petertrr
Copy link
Member

@Cheshiriks please remove binaries from git, instead please either use an HTTP client in Java to download them before test or use maven plugin for downloading. Binaries should be placed in target directory so that they won't show up in Git and could be cleaned up by maven clean

Haven't we decided that for now we will just put the Save in the project, and later we will do the download of the file?

Having a 50MB file in git slows down every fetch operation :( We can add maven configuration later, but downloading from a JVM test seems to be easy enough to implement it right away

### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
@Cheshiriks
Copy link
Member Author

Please add TearDown and TearUp mechanism. No need to download everything and copy everything for thousand of times. Check how @petertrr does it in save-cli

Yes. Added

Copy link
Member

@petertrr petertrr left a comment

Choose a reason for hiding this comment

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

Please move function definitions to more appropriate places, but overall lgtm

### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
# Conflicts:
#	diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
### What's done:
* migrated diktat smoke tests to SAVE-cli mechanism
Closes #1383
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.

Migrate diktat smoke tests to SAVE-cli mechanism
4 participants