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.

Sign up for GitHub

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

Jump to bottom

Add scala tests to CI #872

Closed
wants to merge 7 commits into from
Closed

Add scala tests to CI #872

wants to merge 7 commits into from

Conversation

johnynek
Copy link
Member

@johnynek johnynek commented Feb 9, 2016

This needs to be after #862

@johnynek
Add scala tests to CI
64be52d 
Change-Id: Ib8731522c94f391b855e69198ca1f2e04fd6ce35
@johnynek
Copy link
Member Author

johnynek commented Feb 9, 2016

@damienmg master does not reflect the change i #862 so, this does not pass until it does.

@johnynek
Copy link
Member Author

@damienmg is anything needed for this?

@damienmg damienmg self-assigned this Feb 11, 2016
@damienmg
Copy link
Contributor

LGTM I will merge it tomorrow

Thanks again!

kchodorow
kchodorow reviewed Feb 12, 2016
View reviewed changes
tools/build_defs/scala/scala.bzl
if hasattr(target, "java"):
runtime_jars += target.java.transitive_runtime_deps
#see JavaSkylarkApiProvider.java, this is just the compile-time deps
compile_jars += target.java.transitive_deps
found = True
if not found:
#support http_file pointed at a jar. http_jar uses ijar, which breaks scala macros
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change to "# support" (1 space between # and the comment).

@johnynek
Copy link
Member Author

Something is broken in bazel 0.1.5 with this. Running the tests I get all kinds of errors related to other skylark builds:

st-oscar1:bazel oscar$ bazel test //tools/build_defs/scala/test/...
.....
ERROR: /Users/oscar/oss/bazel/tools/build_defs/dotnet/csharp.bzl:306:3: //external:nunit: no such attribute 'build_file_co
ntent' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/dotnet/csharp.bzl:306:3: //external:nunit: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/d/d.bzl:507:3: //external:dmd_linux_x86_64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/d/d.bzl:507:3: //external:dmd_linux_x86_64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/d/d.bzl:514:3: //external:dmd_darwin_x86_64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/d/d.bzl:514:3: //external:dmd_darwin_x86_64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/go/def.bzl:399:3: //external:golang_linux_amd64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/go/def.bzl:399:3: //external:golang_linux_amd64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/go/def.bzl:406:3: //external:golang_darwin_amd64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/go/def.bzl:406:3: //external:golang_darwin_amd64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/rust/rust.bzl:739:3: //external:rust_linux_x86_64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/rust/rust.bzl:739:3: //external:rust_linux_x86_64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/rust/rust.bzl:747:3: //external:rust_darwin_x86_64: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_rules/rust/rust.bzl:747:3: //external:rust_darwin_x86_64: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/sass/sass.bzl:127:3: //external:libsass: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/sass/sass.bzl:127:3: //external:libsass: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/sass/sass.bzl:135:3: //external:sassc: no such attribute 'build_file_content' in 'new_http_archive' rule.
ERROR: /Users/oscar/oss/bazel/tools/build_defs/sass/sass.bzl:135:3: //external:sassc: missing value for mandatory attribute 'build_file' in 'new_http_archive' rule.
ERROR: package contains errors: tools/build_defs/scala/test.
ERROR: no such package 'external': Package 'external' contains errors.
INFO: Elapsed time: 0.406s
ERROR: Couldn't start the build. Unable to run tests.

Why should errors in other files break this test? For instance:
https://github.com/bazelbuild/bazel/blob/master/tools/build_defs/sass/sass.bzl#L135

does have 'build_file_content' and not a build_file. Is that wrong now?
I wonder if since the other rules aren't in CI they are broken. Any idea what to try?

@johnynek
fix comments, make compile deps non-transitive
5cc0d64 
Change-Id: I700f2670782be5880a7cecd90a8df4e5197c5540
@johnynek
Copy link
Member Author

I guess actually it is the opposite, the code now exercises features not in 0.1.5.

Okay, I addressed the comments and also fixed another issue. I made this more similar to the java rule. I fixed an issue that compilation deps were transitive (unlike java which follows a better practice of requiring declarations).

One issue: I don't see how to access exports in a java rule. I work around this now by using transitive java compile deps, which I assume includes the exports. If there is a way to access that list. I will use it.

This code also builds a small repo of scala code I have.

@kchodorow
Copy link
Contributor

Yeah, I think this is an incompatibility between Bazel at 0.1.5 and the WORKSPACE rules at HEAD. 175b267 added the build_file_content option for new_ repos, and we quickly switched all the built-in Skylark rules over.

I don't know about the exports thing, maybe @damienmg can help with that.

@johnynek
fix comments, compile.sh
97de67b 
Change-Id: Ia4caa7ab704cc7a5544582c6b70c7d257c26f6d8
@johnynek
fix spelling
2098cbe 
Change-Id: If286a9718e036d2bd555baf78600e9208ba5a990
@damienmg
Copy link
Contributor

FYI, this has been merged.

Already answered on the ML for exports thing. And for the
build_file_content, we moved to it because build_file cannot refer to an
external repository (yet?).

On Fri, Feb 12, 2016 at 8:34 PM Kristina notifications@github.com wrote:

Yeah, I think this is an incompatibility between Bazel at 0.1.5 and the
WORKSPACE rules at HEAD. 175b267
175b267
added the build_file_content option for new_ repos, and we quickly switched
all the built-in Skylark rules over.

I don't know about the exports thing, maybe @damienmg
https://github.com/damienmg can help with that.

—
Reply to this email directly or view it on GitHub
#872 (comment).

@johnynek
Copy link
Member Author

Thanks!

bazel-io pushed a commit that referenced this pull request Feb 15, 2016
@johnynek @damienmg
Add scala tests to CI
b5d417d 
--
Change-Id: If286a9718e036d2bd555baf78600e9208ba5a990
Reviewed-on: #872
MOS_MIGRATED_REVID=114563576
@damienmg
Copy link
Contributor

This PR is now live at HEAD!

@damienmg damienmg closed this Feb 15, 2016
@johnynek
Copy link
Member Author

@damienmg Thank you, sir!

damienmg pushed a commit to bazelbuild/rules_scala that referenced this pull request Feb 23, 2016
@johnynek @damienmg
Add scala tests to CI
8248bc0 
--
Change-Id: If286a9718e036d2bd555baf78600e9208ba5a990
Reviewed-on: bazelbuild/bazel#872
MOS_MIGRATED_REVID=114563576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@damienmg damienmg

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@johnynek @damienmg @kchodorow @googlebot