Skip to content

Added a cilium-bsc# test#5

Open
atighineanu wants to merge 54 commits into
fgerling:masterfrom
atighineanu:master
Open

Added a cilium-bsc# test#5
atighineanu wants to merge 54 commits into
fgerling:masterfrom
atighineanu:master

Conversation

@atighineanu
Copy link
Copy Markdown

@atighineanu atighineanu commented Mar 2, 2020

  • added a cilium basic test
  • added a cilium bsc-related test
  • added a kured test
  • moved the .go scripts to live in features/ folder (so they can all be one package)

Comment thread features/cilium/cilium-01.feature
Copy link
Copy Markdown
Owner

@fgerling fgerling left a comment

Choose a reason for hiding this comment

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

I'm looking forward to a call. Parts of it are a direct approve, other parts I'm unsure.

I think we should avoid using stdout for anything, not godog related. Instead, If we really need more output or log files, we can print them on stderr. This way, we won't lose the possibility to generate cucumber reports.
Or we could create a logger that prints into a logfile. Similar to what validator is doing.

Comment thread features/cilium/cilium-01.feature
Comment thread features/goscripts/cilium.go
Comment thread features/goscripts/cilium.go
@fgerling
Copy link
Copy Markdown
Owner

So my request for change would be:

mv features/goscripts/* internal/
rmdir features/goscripts
sed -i 's#features/goscripts#internal#'  main_test.go

@fgerling
Copy link
Copy Markdown
Owner

fgerling commented Mar 19, 2020

Could you rebase your branch against the current master? Maybe we can resolve the conflicting file main_test.go and merge this PR?

@spiarh
Copy link
Copy Markdown
Contributor

spiarh commented Apr 1, 2020

any ETA when we could merge this ?

@fgerling
Copy link
Copy Markdown
Owner

fgerling commented Apr 1, 2020

Due to the amount of new commits that got merge in the remote branch, i have to start from scratch. ETA is tomorrow or friday.

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.

3 participants