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

feat(3266): functional tests for banner API #3273

Merged
merged 11 commits into from
Jan 30, 2025
Merged

feat(3266): functional tests for banner API #3273

merged 11 commits into from
Jan 30, 2025

Conversation

VonnyJap
Copy link
Member

Context

The Banner API has been enhanced to support scope and scopeId in its payload, as implemented in the following PRs.
screwdriver-cd/data-schema#586
screwdriver-cd/models#639

Objective

To validate the functionality of the updated Banner API and ensure it performs as expected, we plan to introduce additional functional tests.

References

#3266

License

I confirm that this contribution is made under a BSD license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@coveralls
Copy link

coveralls commented Jan 28, 2025

Coverage Status

coverage: 95.313%. remained the same
when pulling 2be258d on banner-scope
into d893362 on master.

features/banner.feature Outdated Show resolved Hide resolved
When they create new banner with message "Hello World" and "default" scope
Then they can see that the banner is created with "default" scope
And banner is "updated" when they update the banner with "message" "Some Random Message"
And banner is "not updated" when they update the banner with "scopeId" "1234"
Copy link
Member

Choose a reason for hiding this comment

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

Can we also ensure that scope is immutable?

Copy link
Member Author

Choose a reason for hiding this comment

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

added

And banner is "not updated" when they update the banner with "scope" "PIPELINE"


Background:
Given "calvin" is logged in
# And "calvin" has Screwdriver admin permission
Copy link
Member

Choose a reason for hiding this comment

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

Should we check that only screwdriver admins can create/update banner?

Suggested change
# And "calvin" has Screwdriver admin permission
And "calvin" has Screwdriver admin permission

features/banner.feature Outdated Show resolved Hide resolved
Then they can see that the banner is created with "pipeline" scope
And banner is "updated" when they update the banner with "isActive" "false"
And banner is "not updated" when they update the banner with "scope" "GLOBAL"
Then banner is deleted
Copy link
Member

Choose a reason for hiding this comment

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

Can we add scenarios to ensure, non-admins cannot create/update/delete banner?

Copy link
Member Author

@VonnyJap VonnyJap Jan 29, 2025

Choose a reason for hiding this comment

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

We don't have such headless user right now for the purpose of this test. In order to achieve this, new user needs to be created, so asking that we test this manually as of now and I have verified that only admins can create banner.

Then they can see that the banner is created with "pipeline" scope
And banner is "updated" when they update the banner with "isActive" "false"
And banner is "not updated" when they update the banner with "scope" "GLOBAL"
Then banner is deleted
Copy link
Member

Choose a reason for hiding this comment

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

Can we add tests for below scenario

  1. Non SD admin user, having access to pipeline repo can read pipeline banner
  2. Non SD admin user, not having access to pipeline repo cannot read that pipeline banner
  3. When one or more GLOBAL and PIPELINE banner exists, user can selectively get banners for a specific pipeline

Copy link
Member Author

@VonnyJap VonnyJap Jan 29, 2025

Choose a reason for hiding this comment

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

@sagar1312 - 1 and 2 scenarios will require a different user to login for testing purpose and we do not have such user handy right now, we need to create an extra user for this purpose. So asking to drop this request for now and do the tests manually, which I have done and ensured that only admin users can create banners.

Copy link
Member Author

@VonnyJap VonnyJap Jan 29, 2025

Choose a reason for hiding this comment

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

@sagar1312 - for item 3, I think you were referring to have a test to be able to filter banner by scope and scopeId, so I added a test for that.

Then(/^they can get the banner associated to that pipeline$/, { timeout: TIMEOUT }, function step() {
return request({
url: `${this.instance}/${this.namespace}/banners?scopeId=${this.pipelineId}&scope=PIPELINE`,
method: 'GET',
context: {
token: this.jwt
}
}).then(resp => {
Assert.equal(resp.statusCode, 200);
Assert.equal(resp.body.length, 1);
Assert.equal(resp.body[0].scopeId, this.pipelineId);
});
});

@sagar1312 sagar1312 merged commit 90e5504 into master Jan 30, 2025
2 checks passed
@sagar1312 sagar1312 deleted the banner-scope branch January 30, 2025 17:51
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