Skip to content

Comments

Start testing Sociomantic ocean in pipeline#88

Merged
MartinNowak merged 7 commits intodlang:masterfrom
mihails-strasuns:test-ocean-2
Nov 29, 2017
Merged

Start testing Sociomantic ocean in pipeline#88
MartinNowak merged 7 commits intodlang:masterfrom
mihails-strasuns:test-ocean-2

Conversation

@mihails-strasuns
Copy link
Contributor

@mihails-strasuns mihails-strasuns commented Nov 26, 2017

Recently (sociomantic-tsunami/ocean#321) we have enabled CI testing with vanilla upstream DMD compiler for WIP ocean major branch, as 2.077 provided necessary Throwable.message().

Would appreciate getting it to the list of automatically tested projects ;)

@dlang-bot
Copy link
Collaborator

Thanks for your pull request, @mihails-strasuns! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

@mihails-strasuns mihails-strasuns changed the title Start testing Sociomantic ocean in pipeline #2 Start testing Sociomantic ocean in pipeline Nov 26, 2017
@mihails-strasuns
Copy link
Contributor Author

Good, now fails because ocean is built by default with deprecations-as-errors, which, of course, is not suitable for regression testing with upstream. Will require small tweak in our makefile.

@MartinNowak
Copy link
Member

Nice, will have a look at pre-installing the dependencies with ansible, assuming that they don't change too often.

@mihails-strasuns
Copy link
Contributor Author

Nice, will have a look at pre-installing the dependencies with ansible, assuming that they don't change too often.

Yes, I think at this point they change less often than once a year. Alternatively, you can enable docker on the CI server and script can fetch the very same image we use for our own CI tests.

@MartinNowak
Copy link
Member

Yes, I think at this point they change less often than once a year. Alternatively, you can enable docker on the CI server and script can fetch the very same image we use for our own CI tests.

That could potentially require nested docker support, a.k.a. root on host, and thus rule out some VM slave options. Wouldn't settle on this easily.

@mihails-strasuns
Copy link
Contributor Author

No problem, whatever suits you best. Btw there is no need to add workarounds for deprecation bit - I will update the PR as soon as sociomantic-tsunami/ocean#374 is merged which should be pretty soon :)

sed -i '/^override DFLAGS += -de$/d' Build.mak
git submodule update --init
make d2conv V=1
make test V=1 DVER=2 F=production LDFLAGS=-L./libebtree/usr/lib/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't need LDFLAGS here if dependencies are pre-installed

@mihails-strasuns
Copy link
Contributor Author

Tagged https://github.com/sociomantic-tsunami/ocean/releases/tag/v4.0.0-alpha.1 and pushed commit to switch to it

@MartinNowak
Copy link
Member

Thx, I'd prefer help with the failing tests.
https://ci.dlang.io/blue/organizations/jenkins/dlang-org%2Fci/detail/PR-88/8/pipeline/188#step-342-log-368

ocean.core.Test.TestException@/var/lib/jenkins/dlang_projects@2/sociomantic-tsunami/ocean/test/sysstats/main.d(68): expression 'nan >= 0' evaluates to false
----------------
./src/ocean/core/Enforce.d:416 void ocean.core.Enforce.enforceImpl!(">=", float, int).enforceImpl(lazy Exception, float, int, immutable(char)[], int) [0x6281dd]
./src/ocean/core/Enforce.d:323 void ocean.core.Enforce.enforceImpl!(">=", ocean.core.Test.TestException, float, int).enforceImpl(float, int, immutable(char)[], int) [0x628138]
/var/lib/jenkins/dlang_projects@2/sociomantic-tsunami/ocean/test/sysstats/main.d:68 int 

@mihails-strasuns
Copy link
Contributor Author

Can you run the same pipeline with released 2.077 to see if it is because of compiler or environment?

@mihails-strasuns
Copy link
Contributor Author

mihails-strasuns commented Nov 29, 2017

Though environment is more likely - https://github.com/sociomantic-tsunami/ocean/blob/v3.x.x/test/sysstats/main.d#L68

I guess you have virtualized system where CPU numbers are not accessible?

@mihails-strasuns
Copy link
Contributor Author

mihails-strasuns commented Nov 29, 2017

Ping @Burgos / @nemanja-boric-sociomantic - I think you are more familiar with stats module than me.

@MartinNowak
Copy link
Member

Slaves are using LXD or KVM.

@Burgos
Copy link

Burgos commented Nov 29, 2017

I wonder if the problem is simply because the timing scheduling and CPU counting is different in LXD/KVM than on the bare metal machine. We'll try adding some busy loop there to spend some time in the CPU instead of yielding the control back.

@MartinNowak
Copy link
Member

Stats(nan, nan, nan, 24403968, 8146944, 0.024311)
Reproducibly the uptime diff is 0, while there are as many ticks per second (100), lxcfs rounds them to full seconds.
https://insights.ubuntu.com/2015/03/02/introducing-lxcfs/

@Burgos
Copy link

Burgos commented Nov 29, 2017

Hm, ok. Then we could just sleep for a bit more than one second/two seconds before getting the results. I don't think this will hurt developers much (and they could always make -j to speed the tests). Alternatively, we could just ignore this test on the platforms that behave like this.

@MartinNowak
Copy link
Member

@Burgos see sociomantic-tsunami/ocean#385.

Fixes unreliable stats test
@mihails-strasuns
Copy link
Contributor Author

Updated to 4.0.0-alpha.2, looking at build log

@mihails-strasuns
Copy link
Contributor Author

Green! D-YAML has failed though :)

@Burgos
Copy link

Burgos commented Nov 29, 2017

Such a time to be alive!

@MartinNowak
Copy link
Member

Green! D-YAML has failed though :)

Yes, see dlang/dmd#7315 (comment) :/.

@MartinNowak MartinNowak merged commit f74b1c9 into dlang:master Nov 29, 2017
@MartinNowak
Copy link
Member

Let's hope it doesn't run into any other issues on the KVM host, but since it works on Travis-CI/docker, I wouldn't really expect some.

@mihails-strasuns
Copy link
Contributor Author

@MartinNowak I see. Do you want me to have a look into Jenkins instance taking over what Travis/CircleCI currently do?

@MartinNowak
Copy link
Member

Let's see whether #90 works.

@Burgos
Copy link

Burgos commented Nov 29, 2017

Am I seeing correctly that merging this PR started failing the master? https://ci.dlang.io/blue/organizations/jenkins/dlang-org%2Fci/detail/master/47/pipeline

@MartinNowak
Copy link
Member

@Burgos
Copy link

Burgos commented Nov 29, 2017

The timer loop looks correct to me. I think the problem is that we're checking for equality of the ProcUptime instances: https://github.com/sociomantic-tsunami/ocean/blob/v3.x.x/src/ocean/sys/stats/linux/ProcVFS.d#L209 which includes both uptime and idle but the CPU user calculation uses only a uptime member. https://github.com/sociomantic-tsunami/ocean/blob/v3.x.x/src/ocean/sys/Stats.d#L171

@Burgos
Copy link

Burgos commented Nov 29, 2017

(So the idle progresses, but not the uptime).

@mihails-strasuns
Copy link
Contributor Author

I think for now best course of action would be to disable this one specific test:

diff --git a/Build.mak b/Build.mak
index 11eb0f0..f875f76 100644
--- a/Build.mak
+++ b/Build.mak
@@ -20,6 +20,10 @@ clean += .*.lst
 TEST_FILTER_OUT += \
        $C/test/collectd/main.d
 
+# fails with KVM, temporarily disabled
+TEST_FILTER_OUT += \
+       $C/test/sysstats/main.d
+
 $O/test-%: override LDFLAGS += -lebtree
 
 $O/test-filesystemevent: override LDFLAGS += -lrt

It will fix pipeline for now and we will talk about best way to fix actual test later this week among us.

@Burgos
Copy link

Burgos commented Nov 29, 2017

@mihails-strasuns
Copy link
Contributor Author

#91

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