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

Fixed some tests (bsc#983486). #66

Merged
merged 3 commits into from
Dec 15, 2016
Merged

Fixed some tests (bsc#983486). #66

merged 3 commits into from
Dec 15, 2016

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Dec 15, 2016

Just fixed some tests that does not allow to build the package correctly. Travis does not complain because it uses the old ntp.lense

Josef Reidinger fixed the ntp lense in this commit:

hercules-team/augeas@01e4c42

Basically before the fix it doesn't expect to have restrict records with IPv4 explicitly, as for example:

restrict -4 default

So before the fix -4 was treated as an address and default as an *option what is wrong.

With the new lense, cfa will read the IP version correctly.

IN PROCCESS

The ubuntu repository for augeas is ppa:raphink/augeas, which installs:
Setting up augeas-lenses (1.4.0-0ubuntu1~augeas1~precise4) ...
Setting up libaugeas0 (1.4.0-0ubuntu1~augeas1~precise4) ...

We need updated packages.

Josef Reidinger fixed the ntp lense in this commit:

hercules-team/augeas@01e4c42

Basically before the fix it doesn't expect to have restrict records
with IPv4 explicitly, as for example:

restrict -4 default

So before the fix "-4" was treated as an "address" and "default" as
an option what is wrong.

With the new lense CFA will read the ip version correctly.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.552% when pulling 3c63456 on fix_tests into 81eab9a on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.552% when pulling a7876e5 on fix_tests into 81eab9a on master.

@@ -359,13 +359,13 @@ def ntp_conf(file)

describe "#options" do
it "obtains the options of the record" do
expect(record.options).to eq(%w(default notrap nomodify nopeer))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default is not an option but an address

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.552% when pulling a7876e5 on fix_tests into 81eab9a on master.

@@ -679,7 +683,7 @@
end

it "initializes restrict records" do
expect(subject.restrict_map.size).to eql(4)
expect(subject.restrict_map.size).to eql(3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically it only reads 3 restrict records because they are indexed by the "address" and

restrict -4 default
restrict -6 default

are indexed by the "default" key. With the older lense there was an error which added "-4" as an address that is the reason of having 4 entries previously.

@teclator teclator changed the title Fixed some tests (bsc#983486). [FIX] Fixed some tests (bsc#983486). Dec 15, 2016
@teclator teclator changed the title [FIX] Fixed some tests (bsc#983486). [WIP] Fixed some tests (bsc#983486). Dec 15, 2016
@@ -65,9 +65,9 @@ rake install DESTDIR="%{buildroot}"
%files
%defattr(-,root,root)
%dir %{yast_yncludedir}/ntp-client
%{yast_dir}/clients/*
Copy link
Member

Choose a reason for hiding this comment

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

it can be %{yast_clientdir}/*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@jreidinger
Copy link
Member

LGTM. regarding failing travis only option I see is to install somehow new lense ( from some ubuntu repo or manually ) or wait when @lslezak POC with docker will be used more widely.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.552% when pulling 73064e4 on fix_tests into 81eab9a on master.

@teclator teclator changed the title [WIP] Fixed some tests (bsc#983486). Fixed some tests (bsc#983486). Dec 15, 2016
@lslezak
Copy link
Member

lslezak commented Dec 15, 2016

It's green now! The Docker rocks! 👍

@teclator
Copy link
Contributor Author

Cool!!, thnx @lslezak for make our lives better!! well at least more green ;)

and thnx @jreidinger for review!!

@teclator teclator merged commit c819042 into master Dec 15, 2016
@lslezak lslezak deleted the fix_tests branch December 15, 2016 15:04
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