Skip to content

Added a cilium-bsc# test#5

Open
atighineanu wants to merge 54 commits intofgerling:masterfrom
atighineanu:master
Open

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

Conversation

@atighineanu
Copy link

@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)

Copy link
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.

@fgerling
Copy link
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
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
Contributor

spiarh commented Apr 1, 2020

any ETA when we could merge this ?

@fgerling
Copy link
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