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

testsuite: add longer test descriptions for some of the more complex test scenarios #548

Merged
merged 4 commits into from
Dec 30, 2024

Conversation

cmoussa1
Copy link
Member

@cmoussa1 cmoussa1 commented Dec 16, 2024

Problem

Some of the tests in the testsuite can be somewhat complex and simulate a scenario that might take multiple steps; it would be helpful if descriptions were added that went into more detail than what is described in the one-liner for each test.


This PR makes a number of minor test description improvements in the flux-accounting testsuite, most notably adding some more thorough test descriptions in a couple of the test files explaining what is being tested, particularly for the scenarios that are broken up over multiple tests.

I've also fixed a couple of mistakes in the test description and in already-existing test comments.

Lastly, I've just added a space between the "allow guest access..." tests to match the format of the rest of the testsuite.

Built on top of #547

@cmoussa1 cmoussa1 added the testing issues that deal with testing label Dec 16, 2024
@cmoussa1 cmoussa1 requested a review from wihobbs December 16, 2024 23:26
@@ -156,6 +161,9 @@ test_expect_success 'submit a job using default bank' '
flux cancel ${jobid}
'

# The following two tests ensure job subimssions are rejected when a user
Copy link
Member

Choose a reason for hiding this comment

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

sp nit: submissions

Problem: There is no space between the test that allows guest access to
testexec in a couple of the files in the testsuite.

Add a space between the tests to match the format of the rest of the
testsuite.
Problem: Some of the tests in the testsuite can be somewhat complex and
simulate a scenario that might take multiple steps; it would be helpful
if descriptions were added that went into more detail than what is
described in the one-liner for each test.

Add some more thorough test descriptions in a couple of the test files
explaining what is being tested, particularly for the scenarios that
are broken up over multiple tests.
Problem: There is a test in t1005-max-jobs-limits.t that submits two
jobs to the priority plugin under the same bank using two different
methods: by default and by explicitly setting it with --setattr.
However, the test description is confusing and could be worded better.

Update the test description to more concisely describe what is
happening in the test.
Problem: One of the tests in t1011-job-archive-interface.t has a
lengthy comment explaining in more detail what is being checked in the
test itself, but it is missing an important "not" when describing the
effect on the job usage value for an association.

Fix this comment by adding a "not" to the sentence.
@cmoussa1 cmoussa1 force-pushed the add.test.descriptions branch from 23d7314 to 3f6e1ea Compare December 30, 2024 18:54
@cmoussa1
Copy link
Member Author

Thanks for the review @wihobbs! Force-pushed a fix for the spelling mistake and rebased to catch up after #527; setting MWP here

@mergify mergify bot merged commit 9dbe7fc into flux-framework:master Dec 30, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-passing testing issues that deal with testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants