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

=pro run multi-jvm tests when validating pr #17532

Merged
merged 1 commit into from
May 28, 2015
Merged

Conversation

2m
Copy link
Member

@2m 2m commented May 20, 2015

The essence of this is one more autoplugin MultiNodeWithPrValidation that gets triggered on a project which has both MultiNode and ValidatePullRequest autoplugins enabled.

@akka-ci akka-ci added the validating PR is currently being validated by Jenkins label May 20, 2015
@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels May 20, 2015
@akka-ci
Copy link

akka-ci commented May 20, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2712/

@akka-ci akka-ci added the needs-attention Indicates a PR validation failure (set by CI infrastructure) label May 20, 2015
@akka-ci
Copy link

akka-ci commented May 20, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2711/


implicit class RichBoolean(val b: Boolean) extends AnyVal {
final def apply[A](a: => A): Option[A] = if (b) Some(a) else None
}
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to avoid such very broad conversions (on primitive type, providing an apply()), or if really needed define very near it's use-site.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

It reduces boilerplate when used together with boolean flags that we have quite a few. Therefore it is not possible to add it to a narrower scope.

For example previously we had:

if (CliOption.sbtLogNoFormat) List("-Dakka.test.nocolor=true") else Nil

And now we have instead:

CliOption.sbtLogNoFormat("-Dakka.test.nocolor=true").toList

WDYT?

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels May 21, 2015
@akka-ci
Copy link

akka-ci commented May 21, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2714/

val validateDiagrams = settingKey[Boolean]("Validate generated scaladoc diagrams")

val javadocSettings = if (CliOption.genjavadocEnabled) Seq(
scalacOptions in Compile += "-P:genjavadoc:fabricateParams=true",
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be the failure culprit:

[error] bad option: -P:genjavadoc:fabricateParams=true

Copy link
Member

Choose a reason for hiding this comment

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

the same setting was used previously... so what's different here hm

Copy link
Member

Choose a reason for hiding this comment

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

@2m no need for genjavadocExtraSettings?

Copy link
Member Author

Choose a reason for hiding this comment

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

genjavadocExtraSettings was used before as well.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, the comment was supposed to be "need for genjavadocExtraSettings" it was there but now it's gone

Copy link
Member

Choose a reason for hiding this comment

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

(does not solve the bad option problem we're having though, so maybe a non issue)

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels May 21, 2015
@akka-ci
Copy link

akka-ci commented May 21, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2717/

@ktoso
Copy link
Member

ktoso commented May 21, 2015

LGTM, one nitpick about moving javadoc things out of the scaladoc plugin.

@akka-ci akka-ci added the validating PR is currently being validated by Jenkins label May 21, 2015
@2m
Copy link
Member Author

2m commented May 27, 2015

Made dynamic task execution work with a bit of magic from http://stackoverflow.com/questions/24615783/combining-taskdyn-and-tags-in-sbt thanks to @jsuereth

@akka-ci akka-ci added validating PR is currently being validated by Jenkins and removed tested PR that was successfully built and tested by Jenkins labels May 27, 2015
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels May 27, 2015
@akka-ci
Copy link

akka-ci commented May 27, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2761/

@ktoso
Copy link
Member

ktoso commented May 27, 2015

Finally all green!

@akka-ci akka-ci added the tested PR that was successfully built and tested by Jenkins label May 27, 2015
@akka-ci
Copy link

akka-ci commented May 27, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2762/

@rkuhn
Copy link
Contributor

rkuhn commented May 27, 2015

LGTM

1 similar comment
@patriknw
Copy link
Member

LGTM

@akka-ci akka-ci added validating PR is currently being validated by Jenkins and removed tested PR that was successfully built and tested by Jenkins labels May 28, 2015
@2m
Copy link
Member Author

2m commented May 28, 2015

Added one last commit which removed RootSettings autoplugin and also kept values from tasks to get rid of the

[warn] /home/martynas/projects/akka/project/ValidatePullRequest.scala:217: a pure expression does nothing in statement position; you may be omitting necessary parentheses
[warn]       validationTasks.map(taskKey => Def.task { taskKey.value; () } ).foldLeft(zero) { (acc, current) =>
[warn]

when compiling akka build.

Validated again that no additional tasks are run when project validation is skipped.

Will squash and merge after successful validation.

@akka-ci akka-ci added needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels May 28, 2015
@akka-ci
Copy link

akka-ci commented May 28, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2766/

* addunidoc task via an AutoPlugin that depends on PrValidation and Unidoc autoplugins
* separate cli option logic to a case class
* remove autoplugin for root project
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels May 28, 2015
@akka-ci
Copy link

akka-ci commented May 28, 2015

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://jenkins.akka.io/job/pr-validator-per-commit-jenkins/2769/

2m added a commit that referenced this pull request May 28, 2015
=pro run multi-jvm tests when validating pr
@2m 2m merged commit 0b15a8c into akka:master May 28, 2015
@2m 2m deleted the wip-pr-multi-jvm branch May 28, 2015 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants