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

Granular flake support #126

Merged
merged 4 commits into from
Apr 30, 2016
Merged

Conversation

MylesBorins
Copy link
Contributor

Closes #54

originally flakyness could only be set as a boolean.

Now it can be set in multiple ways

As a boolean: true | false

As a string: version | platform | arch | platform-arch
e.g 'v5' | 'darwin' | 'x86' | 'darwin-x86'

As an array of targets: ['v5', 'v4', 'x86']

As an object for more granular control:

{
  'v4': ['darwin', 'arm'],
  'v5': true,
  'v6': 'linux'
}

Or even as an array of objects:

[{ 'linux': 'v6' }, {'darwin': ['v4', 'v5', 'v6']}]

There is potentially too much granularity here... but it all works
and it is all tested with 100% code coverage so why not.

@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

/cc @orangemocha @mhdawson @jasnell

@MylesBorins MylesBorins force-pushed the granular-flake-support branch 2 times, most recently from f560655 to 3eb8115 Compare April 28, 2016 01:19
@MylesBorins
Copy link
Contributor Author

One more run on master, should be green --> https://ci.nodejs.org/view/Node.js-citgm/job/thealphanerd-smoker/222/

Myles Borins added 4 commits April 28, 2016 17:21
originally flakyness could only be set as a boolean.

Now it can be set in multiple ways

As a boolean: true | false
As a string: version | platform | arch | platform-arch
e.g 'v5' | 'darwin' | 'x86' | 'darwin-x86'
As an array of targets: ['v5', 'v4', 'x86']
As an object for more granular control:

{
  'v4': ['darwin', 'arm'],
  'v5': true,
  'v6': 'linux'
}

Or even as an array of objects:

[{ 'linux': 'v6' }, {'darwin': ['v4', 'v5', 'v6']}]

There is potentially too much granularity here... but it all works
and it is all tested with 100% code coverage so why not.
@MylesBorins MylesBorins force-pushed the granular-flake-support branch from 870578a to a975e4c Compare April 29, 2016 00:21
@jasnell
Copy link
Member

jasnell commented Apr 29, 2016

LGTM

@MylesBorins MylesBorins merged commit a975e4c into nodejs:master Apr 30, 2016
@mhdawson
Copy link
Member

mhdawson commented May 2, 2016

A bit late on my part to comment but here it goes anyway. Overall looks good, but I think this will only works on a subset of linux distros:

 var lsb = require('dotenv').config({path: '/etc/lsb-release', silent: true});

for example on a ppc rhel machine it looks like:

bash-4.1$ cat /etc/lsb-release
LSB_VERSION=base-4.0-noarch:base-4.0-ppc64:core-4.0-noarch:core-4.0-ppc64:graphics-4.0-noarch:graphics-4.0-ppc64:printing-4.0-noarch:printing-4.0-ppc64

as opposed to on an ubuntu machine:

mhdawson@duv-aurora:~/pulls/api/node-eps$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu 14.04.2 LTS"

@MylesBorins
Copy link
Contributor Author

@mhdawson the current logic will make it essentially a no-op on ppc. We are able to already get the specific ppc information from arch and simply filter by ppc, so I'm not sure if there is anything actionable to do with what we've got.

Totally open to adding anything that can improve granularity / control on ppc

@mhdawson
Copy link
Member

mhdawson commented May 2, 2016

@thealphanerd I don't think its limited to ppc, I believe its likely RHEL on x86 and possibly SLES will have contents different than we see on ubuntu.

I checked on one of our SLES 12 x86 and one of our RHEL 71 machines and I don't see /etc/lsb-release at all.

On one x86 SLES 11 machines I see:

dnv-auria:/> cat /etc/lsb-release
LSB_VERSION="core-2.0-noarch:core-3.2-noarch:core-4.0-noarch:core-2.0-x86_64:core-3.2-x86_64:core-4.0-x86_64"

So my more general point is that even on x86 I don't think it will give use what you were hoping for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants