Skip to content

mgr/dashboard: fix orchestrator/02-hosts-inventory.e2e failure#44388

Merged
alfonsomthd merged 1 commit intoceph:masterfrom
rhcs-dashboard:02-host-inventory-fix
Jan 5, 2022
Merged

mgr/dashboard: fix orchestrator/02-hosts-inventory.e2e failure#44388
alfonsomthd merged 1 commit intoceph:masterfrom
rhcs-dashboard:02-host-inventory-fix

Conversation

@nizamial09
Copy link
Member

@nizamial09 nizamial09 commented Dec 23, 2021

I removed the 02-hosts-inventory.e2e file because it is a duplicate
test of one of the test in the 01-hosts.e2e file and fixed the error
from that file.

Also, in the inventory Identify test, we test for an element to be not
visible. According to the latest cypress docs, this should be not.exist
instead of not.visible since the cd-modal will not even be present in
the DOM

Fixes: https://tracker.ceph.com/issues/53499
Signed-off-by: Nizamudeen A nia@redhat.com

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@nizamial09
Copy link
Member Author

jenkins test dashboard

@nizamial09
Copy link
Member Author

jenkins test dashboard cephadm

@alfonsomthd
Copy link
Contributor

Should this be backported to pacific?

Copy link
Member

@epuertat epuertat 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! Small suggestion: instead of decrementing the number of the remaining files, can't we let them as they are? I mean, just skipping the 02, and going to 03, 04, etc.? It's just to minimize the impact of changes: just for not running a single file we are touching 6.

@nizamial09
Copy link
Member Author

nizamial09 commented Dec 23, 2021

Should this be backported to pacific?

@alfonsomthd : Yes.

@nizamial09
Copy link
Member Author

nizamial09 commented Dec 23, 2021

Looks good! Small suggestion: instead of decrementing the number of the remaining files, can't we let them as they are? I mean, just skipping the 02, and going to 03, 04, etc.? It's just to minimize the impact of changes: just for not running a single file we are touching 6.

Yup, I'll do that but it felt odd missing a number between 01 and 03

@nizamial09 nizamial09 force-pushed the 02-host-inventory-fix branch 2 times, most recently from 92b1572 to 8327a0b Compare December 23, 2021 12:21
@epuertat
Copy link
Member

Yup, I'll do that but it felt odd missing a number between 01 and 03

No need: it's actually more important to use this numbering for univocally identifying tests by ID (or number) than keeping the sequence gapless (although we're actually using the numbering for another purpose here: setting the tests execution order). We barely use IDs for tests, but in other environments is pretty common to assign IDs to tests, so that you can see in which branches/releases that specific test unit is failing (Cypress allows you to do that, but in Jenkins/JUnit we're not leveraging that).

I removed the `02-hosts-inventory.e2e` file because it is a duplicate
test of one of the test in the `01-hosts.e2e` file and fixed the error
from that file.

Also, in the inventory Identify test, we test for an element to be not
visible. According to the latest cypress docs, this should be not.exist
instead of not.visible since the cd-modal will not even be present in
the DOM

Fixes: https://tracker.ceph.com/issues/53499
Signed-off-by: Nizamudeen A <nia@redhat.com>
@nizamial09
Copy link
Member Author

Jenkins test make check

@nizamial09
Copy link
Member Author

Jenkins test dashboard

@nizamial09
Copy link
Member Author

Jenkins test api

@alfonsomthd
Copy link
Contributor

@nizamial09 Can you add a comment with the results of the QA run?

@alfonsomthd
Copy link
Contributor

@nizamial09
Copy link
Member Author

jenkins test windows

@nizamial09
Copy link
Member Author

Looks like the teuthology is green 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants