-
Notifications
You must be signed in to change notification settings - Fork 0
update qascsv #3
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
Conversation
- increase folder length, tcase title length - add type (standalone, template) - add custom fields support
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes introduce support for custom fields, test case types, parameter values, and template suffix parameters in the QA Sphere CSV export functionality. The data structures, validation logic, and CSV output were updated accordingly. Test coverage was expanded to include various success and failure scenarios involving custom fields and dynamic string length validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant QASphereCSV
participant CSVExporter
Tester->>QASphereCSV: AddCustomFields(customFields)
Tester->>QASphereCSV: AddTestCases(testCases)
QASphereCSV->>QASphereCSV: Validate test cases (types, custom fields, etc.)
Tester->>QASphereCSV: Export to CSV
QASphereCSV->>CSVExporter: Generate CSV rows (including custom fields)
CSVExporter-->>Tester: Return CSV output
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
qascsv.go (1)
234-248: Lookup of custom-field names is O(N²); cache the names onceEvery time a test-case is validated we iterate over
q.customFieldsfor every entry intc.CustomFields, yielding quadratic behaviour when many cases/fields are involved.- for systemName := range tc.CustomFields { - var found bool - for _, cf := range q.customFields { - if cf.SystemName == systemName { - found = true - break - } - } - if !found { - return errors.Errorf("custom field %s is not defined in QASphereCSV.customFields", systemName) - } - } + nameSet := make(map[string]struct{}, len(q.customFields)) + for _, cf := range q.customFields { + nameSet[cf.SystemName] = struct{}{} + } + for systemName := range tc.CustomFields { + if _, ok := nameSet[systemName]; !ok { + return errors.Errorf("custom field %s is not defined in QASphereCSV.customFields", systemName) + } + }For large data sets this single map construction slashes validation time.
[performance]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
qacsv_test.go(8 hunks)qascsv.go(11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
qacsv_test.go (1)
qascsv.go (11)
CustomField(85-88)CustomFieldTypeDropdown(82-82)CustomFieldTypeText(81-81)TestCase(96-136)Priority(26-26)CustomFieldValue(90-93)Step(65-70)Requirement(44-47)File(56-62)Link(50-53)NewQASphereCSV(149-154)
AndrianBdn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work! let's do few minor fixes and merge this
AndrianBdn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome - thank you @satvik007 !
hi-rai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay to me. Few minor suggestion regarding adding/updating comments as this is a public repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
qascsv.go (2)
82-85: Complete validation for parameter value map entries.The validation rule only validates map keys but not the string values, as noted in a previous review. Values could be oversized or empty and would pass validation.
Apply this fix to validate both keys and values:
- Values map[string]string `json:"values" validate:"required,dive,keys,max=255,endkeys"` + Values map[string]string `json:"values" validate:"required,dive,keys,max=255,endkeys,max=255"`
63-67: Add security validation for file URLs.The File struct contains URL fields that could reference external or unuploaded files, as highlighted in previous comments. Consider adding validation to ensure files are properly uploaded to QA Sphere.
The existing comment mentions users should upload files via the API. Consider adding validation to verify file IDs correspond to actual uploaded files in the QA Sphere project.
🧹 Nitpick comments (1)
qascsv.go (1)
107-107: Consider documenting the increased title length limit.The title length limit was increased from 255 to 511 characters. This should be documented in comments or changelog to inform users of the expanded capability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
qascsv.go(12 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
qascsv.go
[error] 198-205: gofumpt formatting check failed. The file qascsv.go has formatting differences detected by gofumpt.
🔇 Additional comments (5)
qascsv.go (5)
173-189: Good implementation addressing duplicate prevention.The duplicate checking logic properly addresses the previous review concern about preventing duplicate custom field definitions.
259-276: Excellent custom field validation implementation.The validation logic properly ensures that all custom fields referenced in test cases are pre-declared, preventing runtime errors during CSV generation.
202-204: Good default type handling.Defaulting test case type to "standalone" when not specified provides a sensible fallback and maintains backward compatibility.
363-372: 🛠️ Refactor suggestionPotential index mismatch in custom field column generation.
The custom field column assignment uses
customFieldsMap[systemName]as the index, but if a test case doesn't have values for all declared custom fields, this could create sparse arrays with incorrect positioning.Consider initializing the
customFieldColsslice with empty strings to ensure all positions are filled:- customFieldCols := make([]string, len(customFieldsMap)) + customFieldCols := make([]string, len(q.customFields)) + for i := range customFieldCols { + customFieldCols[i] = "" + } for systemName, cfValue := range tc.CustomFields { cfValueJSON, err := json.Marshal(cfValue) if err != nil { return nil, errors.Wrap(err, "json marshal custom field value") } customFieldCols[customFieldsMap[systemName]] = string(cfValueJSON) }Likely an incorrect or invalid review comment.
306-312:Details
❓ Verification inconclusive
Verify custom field header generation logic.
The custom field header format
custom_field_{type}_{systemName}should be consistent with QA Sphere's CSV import expectations.Please verify that the custom field header format matches QA Sphere's documentation:
🌐 Web query:
What is the expected CSV header format for custom fields in QA Sphere CSV imports?💡 Result:
The CSV header format for custom fields in QA Sphere CSV imports follows a specific structure designed to properly import test cases into the QA Sphere Test Management System.
Test Case CSV Structure
QA Sphere requires the following standard columns in the CSV header for test case imports:
- Folder
- Name
- Legacy ID
- Draft
- Priority
- Tags
- Requirements
- Links
- Files
- Preconditions[1]
After these static columns, the CSV should contain dynamic columns for test steps in the following pattern:
- Step1, Expected1
- Step2, Expected2
- And so on, depending on the maximum number of steps in any test case[1]
The steps columns appear as pairs, with each step followed by its expected result column. The number of step columns depends on how many steps your test cases require.
CSV Import Process
To import test cases in CSV format into QA Sphere:
- Open a project and navigate to the Test Cases tab
- Click the three dots menu
- Select Import
- When the CSV Import form appears, select your separator, choose the directory to import into, and pick your file[1]
During the import process, each test case is validated for proper formatting and content. The system automatically assigns the importing user as the author of the test cases and creates them as a contributor to the project after a successful import[1].
The qasphere-csv Go library is also available on GitHub to simplify the creation of CSV files for importing test cases into QA Sphere. This library helps with programmatically creating large projects and facilitates migration from older test management systems by converting exported data into QA Sphere's CSV format[9].
Citations:
- 1: https://docs.qasphere.com/import/
- 2: https://community.atlassian.com/t5/Jira-questions/CSV-Import-of-Custom-Fields/qaq-p/1984017
- 3: https://kb.jetpackcrm.com/knowledge-base/can-i-import-custom-fields-with-csv-importer/
- 4: https://community.atlassian.com/forums/Jira-questions/Import-into-custom-fields-when-importing-CSV/qaq-p/1842612
- 5: https://www.youtube.com/watch?v=w2J7JbXMHZo
- 6: https://www.bigcommerce.com/blog/custom-fields-csv/
- 7: https://help.gohighlevel.com/support/solutions/articles/155000005143-csv-file-format-for-importing-contacts-and-opportunities
- 8: https://discussion.enpass.io/index.php?%2Ftopic%2F14483-import-custom-fields-with-pre-formatted-csv%2F
- 9: https://github.com/Hypersequent/qasphere-csv
- 10: https://support.ecwid.com/hc/en-us/articles/8578428241436-Format-guidelines-for-the-CSV-file
Verify custom field header pattern
The current implementation in qascsv.go (lines 306–312) emits headers as:customFieldHeader := fmt.Sprintf("custom_field_%s_%s", cf.Type, cf.SystemName) rows[0] = append(rows[0], customFieldHeader)QA Sphere’s public CSV import docs list only the standard columns and step/expected pairs and don’t explicitly define how custom fields must be named. Please confirm that
custom_field_{type}_{systemName}aligns with the importer’s requirements (or adjust to the officially supported naming convention).
- File: qascsv.go
- Lines: 306–312
Summary by CodeRabbit
New Features
Bug Fixes
Tests