Skip to content

Conversation

@aoysimmons-GH
Copy link
Collaborator

Feature files - Work-flow

Summary

Fixes / Work for #6146

Changes proposed

Context for reviewers

Validation steps

Feature files - Work-flow
Copy link
Collaborator

@doug-s-nava doug-s-nava left a comment

Choose a reason for hiding this comment

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

I may not be properly understanding the idea of "negative test cases". Some of the cases here seem more like testing how the system works under normal circumstances, rather than making sure the system works as expected when something is wrong. Can we clarify what should be included in a negative tests cases feature, and what should be included elsewhere?

@@ -0,0 +1,180 @@
@6146
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should make sure everyone is on the same page in regards to formatting and file location practices. If you compare this to the work @Bhavna-Ramachandran has done here https://github.com/HHS/simpler-grants-gov/pull/7746/files the file location is different, and the formatting is different as well. Let's try and schedule something this week

by preventing progress and displaying meaningful error messages.

###########################################################################
# Summary
Copy link
Collaborator

Choose a reason for hiding this comment

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

this summary doesn't really explain what is happening with the negative test cases here. I am fine with removing this section, but if it stays in it should reflect what the file is specifically testing

Then the application shell fails to load
And the user is shown an error state
And an error message is displayed indicating the application could not be loaded
# And the user sees the error message "Unable to load application. Please try again later."
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this commented out?

###########################################################################
# 1. Start Application – Shell Fails to Load
###########################################################################
@negative @workflow @start
Copy link
Collaborator

Choose a reason for hiding this comment

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

what do the @workflow and @start annotations mean here?

# 2. Form Navigation – Data Lost or Fails to Navigate
###########################################################################
@negative @workflow @navigation
Scenario: Form navigation retains data when moving between sections
Copy link
Collaborator

Choose a reason for hiding this comment

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

this does not seem to me like a negative scenario, but maybe I'm misunderstanding. I also don't think this is necessarily a meaningful thing to test, as the sections are on the same page and I don't see a way that data could be lost by navigating. We probably should have tests around the usage of the left hand navigation, though, to show that it navigates the user correctly

# And the user sees an error "Invalid file type"

# ###########################################################################
# # 12. Direct Step Bypass Prevention – Bypass Allowed
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't have steps to our application process, so this can be deleted

# And the bypass is not prevented

###########################################################################
# 14. Task Creation – Task Fails to Create
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure what tasks are here, but this can be deleted in any case

And the system displays "Task creation failed"

###########################################################################
# 15. Notification Email – Email Fails to Send
Copy link
Collaborator

Choose a reason for hiding this comment

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

email notifications will be built out later, and we can return to this then. This can be deleted

And an error is logged

###########################################################################
# 16. Workflow Status – Status Fails to Load
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for us this case is more like "unable to load application details" since status will be part of the application details loaded when the application page loads. A test case there is probably a good idea

@@ -0,0 +1,77 @@
@6146
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's hold on to this for later. Feel free to keep a reference to this file, but let's not include it in this PR. I don't think any of these test paths are critical right now. Some of this functionality hasn't been built out yet, and others aren't quite relevant given how our system works

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