Skip to content

Comments

Test suites for inlinel2 and inlinel3#6769

Merged
nqb merged 42 commits intodevelfrom
test/inlinel3
Jan 6, 2022
Merged

Test suites for inlinel2 and inlinel3#6769
nqb merged 42 commits intodevelfrom
test/inlinel3

Conversation

@fdurand
Copy link
Member

@fdurand fdurand commented Dec 13, 2021

Description

Cover test for inlinel3 and inlinel2

Impacts

Tests

Delete branch after merge

YES

@fdurand fdurand added this to the PacketFence-11.2 milestone Dec 13, 2021
@fdurand fdurand requested review from JeGoi and nqb December 13, 2021 20:07
@extrafu extrafu changed the title Test suites for inlinel3 and inlinel3 Test suites for inlinel2 and inlinel3 Dec 14, 2021
Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

I put some single comment during review, I will add here general comments related to the whole test suites:

  1. Could you rebase on devel ?
  2. We need to have variables for each test suite defined in vars/all.yml
  3. When using API calls your JSON payloads contain:
    • default values: could you remove it ?
    • sometimes id key which is not necessary if already present in URL
  4. Could you create a diagram for inline L2 ?
  5. On diagrams, could you mention where is the PacketFence server to clarify ?
  6. [/] I run that scenario on a runner and both test suites failed at Test-if-the-device-is-in-the-ipset-role-ID-2-Guest-role
  7. [/] I and @JeGoi will move some test cases into executor to avoid duplication between both test suites
    => will be done later on a specific PR.

I will add another comment later which more specific to the logic of the scenario

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

Also I suggest to rename test cases without any spaces and uppercase letters because it create long variables (when you want to reuse output from a test case)

EDIT: DONE

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

At the moment, inline test suites don't require other VM than pf VM so they could be part of unit_tests scenario to speed tests.

EDIT: DONE

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

Also we should create a connection profile with an inline filter.

EDIT: dependency to default connection profile in /root/register_node so we keep things like this for now.

@nqb nqb force-pushed the test/inlinel3 branch 2 times, most recently from afbb74d to 253457c Compare December 24, 2021 08:38
fdurand and others added 25 commits January 6, 2022 12:09
We don't configure anymore reg and iso interfaces because they are not
used in this scenario. On top of that, PacketFence removes IP on this
interfaces when we start to configure inline interfaces using API
put back cli login vars removed by bad rebase
@nqb nqb merged commit 7346269 into devel Jan 6, 2022
@nqb nqb deleted the test/inlinel3 branch January 6, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants