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.

Sign up for GitHub

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

Jump to bottom

chore(RHTAPWATCH-1237): Reorganize workspace-manager test suites #75

Merged
merged 1 commit into from
Sep 23, 2024
Merged

chore(RHTAPWATCH-1237): Reorganize workspace-manager test suites #75

merged 1 commit into from
Sep 23, 2024

Conversation

hmariset
Copy link
Contributor

@hmariset hmariset commented Sep 3, 2024

Since functional tests BeforeSuite runs all the tests, separate package for unit and functional tests.

Jira-Url: https://issues.redhat.com/browse/RHTAPWATCH-1237

@hmariset hmariset marked this pull request as draft September 3, 2024 18:34
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch 3 times, most recently from 569f09b to 51f4500 Compare September 4, 2024 15:40
@hmariset hmariset marked this pull request as ready for review September 4, 2024 16:00
@Omeramsc
Copy link
Collaborator

Omeramsc commented Sep 5, 2024

I'm a bit confused about the change.
I can see we have 2 BeforeSuites; are they not affecting each other? We indeed need to separate the functional test's BeforeSuite from the unit tests, but we don't need a BeforeSuite for the unit tests. at least for now.
I also don't see the TestGetNamespacesWithAccess and GetWorkspacesWithAccess tests here, not in the code and not in the CI. why were they removed?

We should see:

  1. a test suite for the functional tests (beforesuite + aftersuite)
  2. tests for TestGetNamespacesWithAccess
  3. tests for GetWorkspacesWithAccess
  4. tests for TestRunAccessCheck
  5. The namespace provisioning test suite Gal added

the tests suites should be separated from each other. meaning the functional tests won't affect the unit tests at all, etc.

Omeramsc
Omeramsc reviewed Sep 5, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch 5 times, most recently from f888269 to c1f5dfd Compare September 5, 2024 16:56
@hmariset hmariset marked this pull request as draft September 5, 2024 17:00
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch 17 times, most recently from fd635d0 to 6985522 Compare September 10, 2024 18:46
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch 19 times, most recently from 4b1406d to eae74ce Compare September 17, 2024 19:36
@hmariset hmariset marked this pull request as ready for review September 17, 2024 19:40
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch from eae74ce to 8c9fafd Compare September 17, 2024 21:22
@hmariset
Copy link
Contributor Author

@Omeramsc @gbenhaim Please review the PR.

Omeramsc
Omeramsc reviewed Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

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

It is quite impossible to give a good review for such a large PR that moves things as well as change them.
either separate the changes or set up an overview call with me (or both)

cmd/main_test.go Show resolved Hide resolved
cmd/main_test.go Show resolved Hide resolved
Omeramsc
Omeramsc reviewed Sep 18, 2024
View reviewed changes
cmd/main_test.go Outdated Show resolved Hide resolved
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch from 8c9fafd to 4775bc9 Compare September 18, 2024 14:39
@hmariset
chore(RHTAPWATCH-1237): Reorganize workspace-manager test suites
8cf96d6 
Since functional tests BeforeSuite runs all the tests, separate
package for unit and functional tests.

Jira-Url: https://issues.redhat.com/browse/RHTAPWATCH-1237
Signed-off-by: Homaja Marisetty <[email protected]>
@hmariset hmariset force-pushed the reorganize-tests-RHTAPWATCH-1237 branch from 4775bc9 to 8cf96d6 Compare September 18, 2024 14:48
Omeramsc
Omeramsc reviewed Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

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

LGTM

Omeramsc
Omeramsc approved these changes Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

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

LGTM

@Omeramsc Omeramsc merged commit 9e43ae5 into konflux-ci:main Sep 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Omeramsc Omeramsc Omeramsc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hmariset @Omeramsc