[sed] Bats tests for sed #2401

james-stocks · 2019-03-29T17:03:33Z

Context

Supersedes #2368

Based on @predominant's work in https://github.com/habitat-sh/core-plans/pull/2377/files but I changed the tests slightly to add assurance that the version of sed that is being executed is the last built sed and not a different package's sed or even the system sed.

Signed-off-by: James Stocks jstocks@chef.io

echohack

Looks good! Thank you for the tests to sed. There's one issue to resolve with this PR. Once that's solved I think it's mergable.

sed/tests/test.bats

gavindidrichsen

Excellent work @james-stocks. 👍

james-stocks · 2019-05-08T08:55:51Z

While these changes are approved, we don't yet have a process for merging commits to base plans outside of a base plan refresh - so we can't immediately merge this.

Signed-off-by: Scott Macfarlane <smacfarlane@chef.io>

smacfarlane · 2019-06-21T17:34:15Z

@james-stocks Would you mind giving this a quick touch-up to meet the testing guidelines? I know you started this before those were in place, so if you don't have the cycles, I'm good with merging this to the refresh branch and adjusting the tests down the road.

[gradle4][gradle5]Fix builds of gradle4 and gradle5

Signed-off-by: Graham Weldon <graham@grahamweldon.com>

…form-0.12.3 [terraform] Update terraform to 0.12.3

* for windows and linux * with amendments according to according to comments in issue habitat-sh#2678 Signed-off-by: Gavin Didrichsen <gavin.didrichsen@gmail.com>

…en/7zip_use_core_pester [7zip] Update windows tests to use new core/pester core plan

* bringing structure inline with [test design document](habitat-sh#2539) * using core/pester Signed-off-by: Gavin Didrichsen <gavin.didrichsen@gmail.com>

…en/fix_dotnet_471_pkg_source_v2 [dotnet-471-dev-pack] VERSION 2: Add Pester tests to habitat installation of dotnet-471-dev-pack

predominant · 2019-06-26T00:46:58Z

sed/plan.sh

@@ -1,4 +1,4 @@
-pkg_name=sed
+pkg_name="sed"


We don't need quotes here.

This fails shellcheck in CI when I remove the quotes

https://buildkite.com/chef-oss/habitat-sh-core-plans-master-verify/builds/1157#03063088-884b-497b-ba89-c8a3dfa9cddb

In sed/plan.sh line 1: -- | pkg_name=sed | ^-- SC2209: Use var=$(command) to assign output (or quote to assign string).

I believe this is because sed is commonly identified as a command that someone is attempting to run. We should add a shellcheck exception for this plan, on this line.

What's the issue with having the quotes?

git grep pkg_name=\" | wc -l tells me there are ~95 core plans with quoted pkg_name values; and I'm wondering if I need to address them all at some point - I can file an issue for that and get them updated, but I'd like to track the reason why it's needed.

I changed to a shellcheck disable comment, but I'm still curious why we don't use quotes to avoid the concern than shellcheck highlights.

Its a false negative in this case. shellcheck thinks you're calling the sed command. There shouldn't be any need to quote a plan name.

sed/tests/test.bats

predominant · 2019-06-26T00:50:19Z

sed/tests/test.sh

@@ -0,0 +1,19 @@
+#!/bin/sh


This test.sh is out of date. We should update this to be more in line with the new testing approach/standard that we have adopted. See examples in plans such as: hugo, terraform, nginx, hapropxy etc. The same applies for the sourceing in test.bats, which is no longer needed when using the new approach.

This test.sh seems to meet the current testing standard and I don't see any sourcing in test.bats

Does this comment refer an older revision of this PR (it did previously have an initial attempt that predated the standard being decided), or am I missing something?

Ah yes I did change the tests a day after this comment was left. I think the line that @predominant commented on remained unchanged, so Github doesn't consider this comment outdated.

Signed-off-by: James Stocks <jstocks@chef.io>

When pkg_name is unquoted like `pkg_name=sed` this causes a shellcheck warning in CI: ``` In sed/plan.sh line 1: pkg_name=sed ^-- SC2209: Use var=$(command) to assign output (or quote to assign string). ``` We don't want quotes here, so instead disable the shellcheck. Signed-off-by: James Stocks <jstocks@chef.io>

james-stocks · 2019-07-01T13:14:31Z

@smacfarlane do you want these changes pointed at refresh/2019q3 instead of master ?

smacfarlane · 2019-07-01T14:39:42Z

@smacfarlane do you want these changes pointed at refresh/2019q3 instead of master ?

@james-stocks Yes, please. Let's incorporate this into the refresh.

james-stocks · 2019-07-01T15:51:22Z

Closing - opened new PR #2755 that points at the refresh branch.

james-stocks added S-awaiting-review Platform:Linux labels Mar 29, 2019

james-stocks requested review from fnichol and smacfarlane as code owners March 29, 2019 17:03

james-stocks mentioned this pull request Mar 29, 2019

[sed] Acceptance tests for sed #2368

Closed

james-stocks force-pushed the james-stocks/sed_tests_bats branch 3 times, most recently from fc5a85c to c1de530 Compare April 1, 2019 09:27

echohack suggested changes Apr 10, 2019

View reviewed changes

sed/tests/test.bats Outdated Show resolved Hide resolved

echohack added Focus:Core Plans Focus:Core-Plans labels Apr 10, 2019

smacfarlane reviewed Apr 10, 2019

View reviewed changes

sed/tests/test.bats Outdated Show resolved Hide resolved

james-stocks force-pushed the james-stocks/sed_tests_bats branch from c1de530 to b450da5 Compare April 12, 2019 13:59

gavindidrichsen self-assigned this May 2, 2019

gavindidrichsen approved these changes May 2, 2019

View reviewed changes

gavindidrichsen mentioned this pull request May 4, 2019

Complete knowledge transfer for new core-plan maintainers #2544

Closed

4 tasks

james-stocks mentioned this pull request May 7, 2019

Add dev policy document for testing standards #2539

Merged

james-stocks added the Action: Do No Merge label May 8, 2019

james-stocks requested a review from echohack May 8, 2019 08:56

predominant removed the S-awaiting-review label Jun 5, 2019

Fix builds of gradle4 and gradle5

0984b44

Signed-off-by: Scott Macfarlane <smacfarlane@chef.io>

smacfarlane and others added 7 commits June 24, 2019 12:26

Merge pull request habitat-sh#2686 from habitat-sh/sm/fix-gradle4-5

3cd1c79

[gradle4][gradle5]Fix builds of gradle4 and gradle5

Update terraform to 0.12.3

ca8b7d8

Signed-off-by: Graham Weldon <graham@grahamweldon.com>

Merge pull request habitat-sh#2720 from predominant/predominant/terra…

4723bd2

…form-0.12.3 [terraform] Update terraform to 0.12.3

Add tests to core/7zip:

ec828fa

* for windows and linux * with amendments according to according to comments in issue habitat-sh#2678 Signed-off-by: Gavin Didrichsen <gavin.didrichsen@gmail.com>

Merge pull request habitat-sh#2630 from gavindidrichsen/gavindidrichs…

c005b2c

…en/7zip_use_core_pester [7zip] Update windows tests to use new core/pester core plan

Test the core/dotnet-471-dev-pack

f4e2f73

* bringing structure inline with [test design document](habitat-sh#2539) * using core/pester Signed-off-by: Gavin Didrichsen <gavin.didrichsen@gmail.com>

Merge pull request habitat-sh#2556 from gavindidrichsen/gavindidrichs…

8cdeeee

…en/fix_dotnet_471_pkg_source_v2 [dotnet-471-dev-pack] VERSION 2: Add Pester tests to habitat installation of dotnet-471-dev-pack

predominant suggested changes Jun 26, 2019

View reviewed changes

james-stocks force-pushed the james-stocks/sed_tests_bats branch 3 times, most recently from 22931a8 to 9c70c0c Compare June 26, 2019 08:41

[sed] Add tests

9c70c0c

Signed-off-by: James Stocks <jstocks@chef.io>

gavindidrichsen removed their assignment Jun 26, 2019

james-stocks force-pushed the james-stocks/sed_tests_bats branch from 08be889 to 18a76f5 Compare July 1, 2019 13:09

james-stocks changed the base branch from master to refresh/2019q3 July 1, 2019 15:49

james-stocks closed this Jul 1, 2019

james-stocks mentioned this pull request Jul 1, 2019

[sed] Add tests #2755

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sed] Bats tests for sed #2401

[sed] Bats tests for sed #2401

james-stocks commented Mar 29, 2019 •

edited

Loading

echohack left a comment

gavindidrichsen left a comment •

edited

Loading

james-stocks commented May 8, 2019

smacfarlane commented Jun 21, 2019

predominant Jun 26, 2019

james-stocks Jun 26, 2019

predominant Jun 27, 2019

james-stocks Jun 27, 2019

james-stocks Jul 1, 2019

predominant Jul 1, 2019

predominant Jun 26, 2019

james-stocks Jul 1, 2019

james-stocks Jul 1, 2019

james-stocks commented Jul 1, 2019

smacfarlane commented Jul 1, 2019

james-stocks commented Jul 1, 2019 •

edited

Loading

[sed] Bats tests for sed #2401

[sed] Bats tests for sed #2401

Conversation

james-stocks commented Mar 29, 2019 • edited Loading

Context

echohack left a comment

Choose a reason for hiding this comment

gavindidrichsen left a comment • edited Loading

Choose a reason for hiding this comment

james-stocks commented May 8, 2019

smacfarlane commented Jun 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-stocks commented Jul 1, 2019

smacfarlane commented Jul 1, 2019

james-stocks commented Jul 1, 2019 • edited Loading

james-stocks commented Mar 29, 2019 •

edited

Loading

gavindidrichsen left a comment •

edited

Loading

james-stocks commented Jul 1, 2019 •

edited

Loading