Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI e2e testing framework #7350

Merged
merged 57 commits into from
Jun 12, 2023
Merged

UI e2e testing framework #7350

merged 57 commits into from
Jun 12, 2023

Conversation

satkunas
Copy link
Contributor

@satkunas satkunas commented Nov 7, 2022

Description

Integrate cypress end-to-end testing framework

Impacts

Venom workflows
CI/CD results

NEW Package(s) required

RHEL

xorg-x11-server-Xvfb gtk2-devel gtk3-devel libnotify-devel GConf2 nss libXScrnSaver alsa-lib

Debian

libgtk2.0-0 libgtk-3-0 libgbm-dev libnotify-dev libgconf-2-4 libnss3 libxss1 libasound2 libxtst6 xauth xvfb

Delete branch after merge

YES

Checklist

(REQUIRED) - [yes, no or n/a]

  • Document the feature
  • Add unit tests
  • Add acceptance tests (TestLink)

NEWS file entries

(REQUIRED, but may be optional...)

New Features

  • Adding end-to-end testing framework to UI for CI/CD pipelines

.gitlab-ci.yml Show resolved Hide resolved
t/html/pfappserver/Makefile Outdated Show resolved Hide resolved
t/html/pfappserver/Makefile Outdated Show resolved Hide resolved
t/html/pfappserver/README.md Show resolved Hide resolved
t/venom/scenarios/pfappserver/playbooks/run_tests.yml Outdated Show resolved Hide resolved
t/venom/vars/all.yml Show resolved Hide resolved
@nqb
Copy link
Contributor

nqb commented May 31, 2023

Difference between pfappserver_configurator and configurator test suites

It looks like there are differences between pfappserver_configurator and configurator (existing) test suites.

  • Step 1: configurator will configure dhcp-listener, registration and isolation interfaces, DNS servers
  • Step 2: configurator will configure alerting and send an email
  • Post-configurator: configurator will use new admin password defined at step 2 to login against API

[X] Packaging

We need to package t/html/ directory in packetfence-test package. Not sure if this is the case at the moment.

EDIT: packaging checked and t/html is part of packetfence-test on RPM and Deb packages.

Copy link
Contributor

@JeGoi JeGoi left a comment

Choose a reason for hiding this comment

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

Some modifications/answers on my sides

.gitlab-ci.yml Outdated Show resolved Hide resolved
.gitlab-ci.yml Outdated Show resolved Hide resolved
t/venom/Makefile Outdated Show resolved Hide resolved
@echo "make test-x11: run tests interactively (w/ X11 \$$DISPLAY)"
@echo ""
@echo "make [target] DEBUG=cypress:* (see https://docs.cypress.io/guides/references/troubleshooting#Log-sources)"
@echo "make [target] BROWSER=firefox:nightly"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@echo "make [target] BROWSER=firefox:nightly"
@echo "make [target] BROWSER= chrome" # or for ex: firefox:nightly

To be consistant with default values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default was changed, firefox is preferred due to lower memory usage. It also gives us better coverage that will prevent firefox-only issues in the future.

'cypress/specs/e2e/*-configurator/**/*.cy.{js,jsx,ts,tsx}',
],
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as the other one => t/html/pfappserver/cypress/config/cypress.config-configuration.js
Duplicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a stub for differentiation so it's more explicit for the next person to setup a new scenario using the same framework.

module.exports = {
// defaultCommandTimeout: 10000, // 10s
e2e: {
baseUrl: 'https://localhost:1443',
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it supposed to be variables? or there is an override system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cypress uses NodeJS, so this is what's expected

Copy link
Contributor Author

@satkunas satkunas Jun 2, 2023

Choose a reason for hiding this comment

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

I misunderstood which line you were referring to. I understand now.

This is the default value. Our vars are injected in the scenario through make using the BASE_URL environmental value.

t/venom/scenarios/pfappserver/playbooks/run_tests.yml Outdated Show resolved Hide resolved
Comment on lines +2 to +16
/*
cy.visit('https://localhost:1443/admin#/login').then(() => {
cy.readFile('/usr/local/pf/conf/unified_api_system_pass').then((password) => {
cy.get('form input#username').first().type('system')
cy.get('form input#password').first().type(password)
cy.get('form button[type="submit"]').first().click()
})
})
*/
return cy.pfUnifiedSystemPassword().then(password => {
return cy.request('POST', '/api/v1/login', { username: 'system', password }).then(response => {
return window.localStorage.setItem('user-token', response.body.token)
})
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using API or the browser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The configurator is using the Browser, which doesn't require /login

This was created during the PoC to provide a support helper for writing tests. I left it there to help with future development.

@satkunas satkunas requested review from nqb and JeGoi June 2, 2023 18:12
@nqb
Copy link
Contributor

nqb commented Jun 6, 2023

I resolved some conversation.
Two items remaining for me:

  • Difference between pfappserver_configurator and configurator test suites
  • Tag and Tags

Copy link
Contributor

@JeGoi JeGoi left a comment

Choose a reason for hiding this comment

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

Move t/venom/scenarios/pfappserver/README.md to t/venom/test_suites/pfappserver_configurator/TESTSUITE.md to be consistant with the other tests.

t/venom/scenarios is more for ansible to prepare the environment and the test and it can be shared with multiple test_suites.

@satkunas
Copy link
Contributor Author

satkunas commented Jun 6, 2023

@nqb repo rebased w/ devel.

Copy link
Contributor

@JeGoi JeGoi left a comment

Choose a reason for hiding this comment

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

Ok, looks good to me.

@nqb
Copy link
Contributor

nqb commented Jun 7, 2023

Running a pipeline to see if artifacts are saved correctly: https://gitlab.com/inverse-inc/packetfence/-/pipelines/891933439

@nqb
Copy link
Contributor

nqb commented Jun 7, 2023

Regarding Difference between pfappserver_configurator and configurator test suites, @satkunas is aware that test suites are different. We will keep things as is. Once we want to deprecate configurator test suite, we will extend what pfappserver_configurator is doing.

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.

@satkunas, I identified few issues:

  • We need to configure 2nd interface (eth1) as management during configurator and not 1st interface (eth0). Otherwise, it can cause issues because iptables will reject SSH requests on eth1
  • Artifacts were not correctly pulled using Ansible because hostname is changed during configurator. This issue should be fixed now
  • I'm not able to see if screenshots are available in GitLab because it looks like current Cypress test doesn't generate screenshots, could you tell me how I can enable this ?
  • There is a firefox error when we run tests on Debian (I provided logs in DM)

Also I opened #7704 because I got it while testing.

@satkunas
Copy link
Contributor Author

satkunas commented Jun 7, 2023

@nqb

We need to configure 2nd interface (eth1) as management during configurator and not 1st interface (eth0). Otherwise, it can cause issues because iptables will reject SSH requests on eth1

Adjustments have been made

I'm not able to see if screenshots are available in GitLab because it looks like current Cypress test doesn't generate screenshots, could you tell me how I can enable this ?

This is automatic. By default at least 1 screenshot should be created w/ a failed test. This is where I mentioned I had some trouble extracting the them from the VM.

There is a firefox error when we run tests on Debian (I provided logs in DM)

I added a -headless Cypress command-line flag for firefox. My reasoning is based on libva error: vaGetDriverNameByIndex() failed with unknown libva error, driver_name = (null) in venom.log

If the test still fails, we may need to revert back to using the default browser.

diff --git a/t/venom/test_suites/pfappserver_configurator/run_e2e.yml b/t/venom/test_suites/pfappserver_configurator/run_e2e.yml
index 7d0c2ece7a..f8c498b75c 100644
--- a/t/venom/test_suites/pfappserver_configurator/run_e2e.yml
+++ b/t/venom/test_suites/pfappserver_configurator/run_e2e.yml
@@ -5,7 +5,6 @@ testcases:
   steps:
   - type: html_e2e
     base_url: "{{.html.pfappserver_baseurl}}"
-    browser: firefox
     config_file: "cypress/config/cypress.config-configurator.js"
     tag: "pfappserver_configurator"
     info:

@satkunas satkunas requested a review from nqb June 7, 2023 16:49
@nqb
Copy link
Contributor

nqb commented Jun 8, 2023

Running a new pipeline to see if it's better: https://gitlab.com/inverse-inc/packetfence/-/pipelines/893321590

@nqb nqb merged commit 33aa736 into devel Jun 12, 2023
@nqb nqb deleted the feature/ui-e2e branch June 12, 2023 08:16
nqb added a commit that referenced this pull request Jun 12, 2023
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.

None yet

3 participants