Skip to content

Conversation

@Zerpet
Copy link
Member

@Zerpet Zerpet commented Apr 7, 2022

Summary Of Changes

Two significant changes affected our code base:

  1. The table DSL i.e. DescribeTable and Entry have been moved to
    top-level. It was not required to import extensions/table anymore
  2. Changes to CLI flag arguments to randomise all specs

There was a function name conflict because our code base and Ginkgo
define the function Label(). Due to the dot-import, it caused a name
conflict. This was resolved by aliasing our module.

Disabled metrics in integration tests to avoid MacOS firewall popup
everytime we run integration tests. We do not observe or scrape metrics
in the integration tests at the moment. In the future, we could bind to
localhost if needed.

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed

Additional Context

Unit and integration tests are green.

Local Testing

Please ensure you run the unit, integration and system tests before approving the PR.

To run the unit and integration tests:

$ make unit-tests integration-tests

You will need to target a k8s cluster and have the operator deployed for running the system tests.

For example, for a Kubernetes context named dev-bunny:

$ kubectx dev-bunny
$ make destroy deploy-dev
# wait for operator to be deployed
$ make system-tests

@Zerpet Zerpet marked this pull request as draft April 8, 2022 09:03
@Zerpet Zerpet marked this pull request as ready for review April 8, 2022 16:15
@Zerpet
Copy link
Member Author

Zerpet commented Apr 8, 2022

System tests seem to be a bit flaky in Actions. System tests run fine locally.

@Zerpet
Copy link
Member Author

Zerpet commented Apr 19, 2022

Right so "resolve conflicts" button merges the main branch into the feature branch 🤦‍♂️ I'll resolve them locally and force push.

Two significant changes affected our code base:

1. The table DSL i.e. `DescribeTable` and `Entry` have been moved to
   top-level. It was not required to import `extensions/table` anymore
1. Changes to CLI flag arguments to randomise all specs

There was a function name conflict because our code base and Ginkgo
define the function `Label()`. Due to the dot-import, it caused a name
conflict. This was resolved by aliasing our module.

Disabled metrics in integration tests to avoid MacOS firewall popup
everytime we run integration tests. We do not observe or scrape metrics
in the integration tests at the moment. In the future, we could bind to
localhost if needed.

Small refactor on getting the configuration to use a library function,
instead of a home-made one.

Signed-off-by: Aitor Perez Cedres <[email protected]>
Copy link
Contributor

@DanielePalaia DanielePalaia left a comment

Choose a reason for hiding this comment

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

Looks good!

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