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

CATROID-449 move end2end tests against share to nightly runs #3451

Merged
merged 1 commit into from
Feb 11, 2020
Merged

CATROID-449 move end2end tests against share to nightly runs #3451

merged 1 commit into from
Feb 11, 2020

Conversation

84n4n4
Copy link
Contributor

@84n4n4 84n4n4 commented Jan 29, 2020

No description provided.

@84n4n4
Copy link
Contributor Author

84n4n4 commented Jan 29, 2020

@jampo can you please review?

if this is fine for you, can we move this job over to the jenkins root (out of my lab package). and is there any possibility to have test results posted to specific slack channels? (eg. catrobat share and scratch converter)
current test job is here: https://jenkins.catrob.at/job/lab/job/Thomas_Hirsch/job/nightly%20end2end%20tests/

thank you!

Copy link
Contributor

@DinosaurierRex DinosaurierRex left a comment

Choose a reason for hiding this comment

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

As discussed with @84n4n4 LGTM 👍

Maybe @jampo or @AndreasFinsterbusch can have a look at the jenkins file?

Copy link
Contributor

@AndreasFinsterbusch AndreasFinsterbusch left a comment

Choose a reason for hiding this comment

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

The rest is fine for me +1

and is there any possibility to have test results posted to specific slack channels? (eg. catrobat share and scratch converter)

yes, you can report the build result to a channel. just add:
notifyChat(['#channelName1', '#ChannelName2']) into the post{always{}} block

stages {
stage('End to end tests requiring share authentication') {
steps {
catchError(buildResult: 'FAILURE' ,stageResult: 'FAILURE') {
Copy link
Contributor

Choose a reason for hiding this comment

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

this catchError block is unnecessary and should be removed!

@84n4n4
Copy link
Contributor Author

84n4n4 commented Feb 7, 2020

changes suggested by @AndreasFinsterbusch applied, please recheck!

thank you!

Copy link
Member

@wslany wslany left a comment

Choose a reason for hiding this comment

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

Following https://www.selfelected.com/code-reviewing-comments/ =>

PRAISE: This is great!!

CHECK: @84n4n4 will you still integrate the feedback from @AndreasFinsterbusch ?

FIX: These tests shall not have the word "nightly" anywhere in their name. These tests may have to be executed nightly alright, but they will also have to be executed at other times and on other occasions. Please rename Jenkisfile.NightlyNetworkTests to Jenkisfile.OutgoingNetworkCallTests or even better, Jenkisfile.SystemTests or something else that is fitting --- I'm not sure what the best name here would be. We already have other tests that should be kept separate from the tests for developers, though they are currently still included, e.g., the sensor box tests, or soon the robot arm tests etc. If so, will these be handled by the same Jenkisfile test suite? If yes, I would suggest to rename it already now to something like Jenkisfile.SystemTests --- but I'm willing to go for Jenkisfile.OutgoingNetworkCallTests if that will not be the case of course. Please let me know. Anyway, the "nightly" should be eliminated.

@AndreasFinsterbusch
Copy link
Contributor

changes suggested by @AndreasFinsterbusch applied, please recheck!

thank you!

PRAISE: Now it is fine for me!

@84n4n4
Copy link
Contributor Author

84n4n4 commented Feb 8, 2020

@AndreasFinsterbusch is there any possibility to send notifications only when the tests fail? (and do nothing if the run is a success)

notifyChat now commented out for the time.

@AndreasFinsterbusch
Copy link
Contributor

AndreasFinsterbusch commented Feb 8, 2020

@AndreasFinsterbusch is there any possibility to send notifications only when the tests fail? (and do nothing if the run is a success)

notifyChat now commented out for the time.

yes, currently it runs in the always section,
you can create a another section on the same level with a other condition, ( eg. failure, unsuccessful, changed)

see: https://jenkins.io/doc/book/pipeline/syntax/#post

@84n4n4
Copy link
Contributor Author

84n4n4 commented Feb 11, 2020

@AndreasFinsterbusch kannst du bitte hier mal drüberschaun:
https://jenkins.catrob.at/job/lab/job/Thomas_Hirsch/job/nightly%20end2end%20tests/23/console

hast du eine ahnung warum der job das git repo nicht mehr pullen kann?

danke!

@AndreasFinsterbusch
Copy link
Contributor

@AndreasFinsterbusch kannst du bitte hier mal drüberschaun:
https://jenkins.catrob.at/job/lab/job/Thomas_Hirsch/job/nightly%20end2end%20tests/23/console

hast du eine ahnung warum der job das git repo nicht mehr pullen kann?

danke!

Sollte wieder gehen, es wurde ein lock im Workspace nicht mehr freigeben. Weiß leider nicht wie es zu diesem fehler gekommen ist. Hoffe es tritt nicht mehr auf.

Mir ist aufgefallen ihr habt paar deprecated warnings im log zb:
15:41:32 w: /home/catroid/jenkins_slave/workspace/lab/Thomas_Hirsch/nightly/catroid/src/main/java/org/catrobat/catroid/ui/ImportLaunchers.kt: (74, 45): 'getInstance(): ProjectManager!' is deprecated. Deprecated in Java

15:42:01 > Task :catroid:createCatroidDebugAndroidTestCoverageReport 15:42:01 15:42:01 Deprecated Gradle features were used in this build, making it incompatible with Gradle 6.0. 15:42:01 Use '--warning-mode all' to show the individual deprecation warnings. 15:42:01 See https://docs.gradle.org/5.6.4/userguide/command_line_interface.html#sec:command_line_warnings

@wslany
Copy link
Member

wslany commented Feb 11, 2020

Mir ist aufgefallen ihr habt paar deprecated warnings im log zb:

@84n4n4 Thomas will you fix these? Please tell me via slack when I can merge the PR. LGTM, but I'm not sure about the deprecation warnings, please decide and let me know.

@84n4n4
Copy link
Contributor Author

84n4n4 commented Feb 11, 2020

die deprecation warnings kommen seit dem dependency injection PR, da wurde "getInstance" als deprecated annotiert um zu verhindern dasses weiter in neuem code eingebaut wird. dabei wurden natürlich noch nicht alle occurrences eliminiert (da projectManager ja wirklich überall in der codebase angegriffen wird).

hat also nichts mit diesem PR zu tun, und sollte eigentlich in allen anderen suits auch reported werden (abhängig vom eingestellten warning level)

@wslany wslany merged commit 61e94ff into Catrobat:develop Feb 11, 2020
@wslany
Copy link
Member

wslany commented Feb 11, 2020

Thanks!!

Koell pushed a commit to Koell/Catroid that referenced this pull request Feb 28, 2020
CATROID-449 move end2end tests against share to nightly runs
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.

4 participants