search

FRosner / spawncamping-dds

Data-Driven Spark allows quick data exploration based on Apache Spark.
Other
28 stars 15 forks source link

#227 beware of platform precision problems in unit test #256

Closed Husterknupp closed 8 years ago

Husterknupp commented 9 years ago

thanks for the introduction señor :dancer:

FRosner commented 9 years ago

Shoot, @Husterknupp. My sbt task for automatically uploading the master branch as a snapshot jar to S3 caused the build to fail. So we have two options. Either, you recreate the pull request on a feature branch instead (so the build does not trigger the upload), or I need to find a way to tell sbt that it is a pull request that is being build.

Travis CI Documentation

Rather than test the commits from the branches the pull request is sent from, we test the merge between the origin and the upstream branch.

The environment variable ${TRAVIS_PULL_REQUEST} is set to "false" when the build is for a normal branch commit. When the build is for a pull request, it will contain the pull request’s number.

script:
  - '[ "${TRAVIS_PULL_REQUEST}" = "false" ] && bundle exec rake tests:integration || false'
Husterknupp commented 9 years ago

Why did the automatic uploading fail? Was it because Travis' output was only the merged lines of code?

FRosner commented 9 years ago

It is because for uploading (which anyway should only happen to merged branches and not pull requests), you need a key. This key is for obvious reasons only visible to builds triggered directly from my repository. Otherwise you could gain access to my S3 bucket.

I will fix it and would like you to recreate the pull request then to make sure that it worked :-)

Sent from my phone. Please excuse my brevity.

Benjamin Schandera notifications@github.com wrote:

Why did the automatic uploading fail? Was it because Travis' output was only the merged lines of code?

— Reply to this email directly or view it on GitHub.

FRosner commented 8 years ago

@Husterknupp can you please rebase your master against mine so the pull request gets reissued with the fix from #257?

If you cannot do it due to time constraints, let me know.

Husterknupp commented 8 years ago

Hey @FRosner thanks for keeping me updated. I will find time this or the next week.

FRosner commented 8 years ago

What's the status on this guy @Husterknupp?

Husterknupp commented 8 years ago

Hey @FRosner I lost track a bit :palm_tree:

I rebased my master against your master. Also all unit tests passed. I dont see Travis executing his checks.. I guess you need to trigger them manually?

FRosner commented 8 years ago

Did you push your changes? If you rebase, you might have to force push.

Then the PR should automatically pick up the changes to your branch?

Husterknupp commented 8 years ago

Yessir. I rebased my master against your master and had no merge conflicts. After that I force pushed my local master to my remote master.

find a863eeeb in https://github.com/Husterknupp/spawncamping-dds/commits/master

FRosner commented 8 years ago

And Travis is checking, no?

Husterknupp commented 8 years ago

Github is saying below .. including yellow bubble

Some checks haven’t completed yet 1 expected check Required continuous-integration/travis-ci — Waiting for status to be reported

FRosner commented 8 years ago

"Some check haven't completed yet" usually means, that Travis is still running... I am not sure. But it might be because you did something funny when rebasing, because instead of putting your commit on top you put mine on top and committed them as yours.

FRosner commented 8 years ago

Let's do a screen sharing to resolve this problem tomorrow? Or you can just reset your branch and recommit, as it is only a small change. Or I can simply cherry-pick your commit if this works for you.

LMK

Husterknupp commented 8 years ago

wait now I see Travis executing a check. I think because my git status said something about being not up-to-date, after I rebased against your master, I again rebased against my master before I force pushed. Which put my test fix commit not on top of your master but rather back at my master + 1. Forgot to put on my smarty pants

Anyway, it should be good now.

codecov-io commented 8 years ago

Current coverage is 94.80%

Merging #256 into master will not affect coverage as of da997f8

@@            master   #256   diff @@
=====================================
  Files           20     20       
  Stmts         1193   1193       
  Branches       147    147       
  Methods          0      0       
=====================================
  Hit           1131   1131       
  Partial          0      0       
  Missed          62     62

Review entire Coverage Diff as of da997f8

Powered by Codecov. Updated on successful CI builds.

RPCMoritz commented 8 years ago

Good job!

On Wed, Jan 27, 2016 at 8:50 AM, Frank Rosner notifications@github.com wrote:

Merged #256 https://github.com/FRosner/spawncamping-dds/pull/256.

— Reply to this email directly or view it on GitHub https://github.com/FRosner/spawncamping-dds/pull/256#event-527693811.