Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

tests: test fleetd with extra configuration options #1556

Merged
dongsupark merged 2 commits into from
May 20, 2016
Merged

tests: test fleetd with extra configuration options #1556

dongsupark merged 2 commits into from
May 20, 2016

Conversation

@kayrus
Copy link
Contributor

@kayrus kayrus commented Apr 18, 2016

covers #1418

/cc @tixxdz

@kayrus kayrus force-pushed the kayrus/cache_func_tests branch 2 times, most recently from fc31f75 to 76e459d Compare April 18, 2016 13:46
@tixxdz
Copy link
Contributor

tixxdz commented Apr 18, 2016

cc @hectorj2f

@kayrus thank you! so we wait for replies on this #1418 (comment) we see, we test it, and if it's hard to achieve note 1) of that comment, we merge this one and we follow up with PR #1418

Thanks!

tixxdz
tixxdz reviewed Apr 19, 2016
View reviewed changes
functional/test Outdated
TESTS_RETURN_CODE_1=${PIPESTATUS[0]}

# Run tests with cache enable on fleetd side
export TEST_ENV="enable_unitstate_cache=true"
Copy link
Contributor

@tixxdz tixxdz Apr 19, 2016

Choose a reason for hiding this comment

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

Could you rename from TEST_ENV to FLEETD_TEST_ENV ?

@tixxdz
Copy link
Contributor

tixxdz commented Apr 19, 2016

@kayrus thanks for the PR!

Currently the output is:
-- PASS: TestKnownHostsVerification (enable_unitstate_cache=true) (6.02s)

Would it be better to switch to:
-- PASS: TestKnownHostsVerification (fleet.conf=[enable_unitstate_cache=true]) (6.02s)
?

lgtm overall! tests pass, so waiting for @hectorj2f before proceeding.

@tixxdz tixxdz added this to the v0.13.0 milestone Apr 19, 2016
@tixxdz tixxdz mentioned this pull request Apr 21, 2016
Closed
@dongsupark
Copy link

dongsupark commented May 11, 2016

BTW this PR needs to be rebased too.

@dongsupark dongsupark force-pushed the kayrus/cache_func_tests branch from 76e459d to 7f20a17 Compare May 18, 2016 14:46
@dongsupark
Copy link

dongsupark commented May 18, 2016 •
edited
Loading

I rebased this PR, as @kayrus is on vacation. Also I renamed a variable from TEST_ENV to FLEETD_TEST_ENV.

@dongsupark dongsupark force-pushed the kayrus/cache_func_tests branch from 7f20a17 to 8d1a296 Compare May 18, 2016 15:07
@dongsupark
Copy link

dongsupark commented May 18, 2016

Oops, I had missed a change in functional/util/config.go. So I updated it by renaming TEST_ENV to FLEETD_TEST_ENV.

@dongsupark dongsupark force-pushed the kayrus/cache_func_tests branch from 8d1a296 to 3a11907 Compare May 18, 2016 16:01
functional: run go test just once, without unit state cache option
5cc9d3b 
As the unit state cache is going to be obsolete, functional test does
not need to run go test with unit state cache enabled. So let's remove
the second run for now.
@dongsupark dongsupark force-pushed the kayrus/cache_func_tests branch from 3a11907 to 5cc9d3b Compare May 18, 2016 16:39
@dongsupark
Copy link

dongsupark commented May 18, 2016

Fixed more bugs.

  • Fixed a wrong pattern : from PASS|SKIP|SKIP to PASS|SKIP|FAIL.
  • Added a prefix fleet.conf= to output strings, like @tixxdz suggested.
  • Disabled the second round of test, as unit state cache is going to be obsolete.

@dongsupark dongsupark changed the title tests: test fleed with extra configuration options tests: test fleetd with extra configuration options May 19, 2016
@dongsupark
Copy link

dongsupark commented May 19, 2016

If no one objects, I'll merge it tomorrow.

@dongsupark dongsupark merged commit 510103f into coreos:master May 20, 2016
dongsupark pushed a commit that referenced this pull request May 20, 2016
Merge pull request #1556 from endocode/kayrus/cache_func_tests
3a974af 
tests: test fleetd with extra configuration options
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

area/testing kind/enhancement priority/P1 reviewed/lgtm

Projects

None yet

Milestone

v0.13.0

Development

Successfully merging this pull request may close these issues.

3 participants

@kayrus @tixxdz @dongsupark