Skip to content

Testdeletecontext fails#418

Open
Ansita20 wants to merge 6 commits into
microcks:masterfrom
Ansita20:testdeletecontext-fails
Open

Testdeletecontext fails#418
Ansita20 wants to merge 6 commits into
microcks:masterfrom
Ansita20:testdeletecontext-fails

Conversation

@Ansita20
Copy link
Copy Markdown

Summary

This PR fixes Issue #417 by removing the brittle relative path dependency in TestDeleteContext and correcting a typo in the application entrypoint.

Changes Made

  • Fixed entrypoint typo in main.go

    • Changed:

      cmd.NewCommad()
    • To:

      cmd.NewCommand()
  • Updated cmd/context_test.go

    • Replaced the hardcoded path:

      ./testdata/local.config
    • With a temporary isolated file using:

      filepath.Join(t.TempDir(), "local.config")
    • Added the required path/filepath import.

Verification

  • Created branch: testdeletecontext-fails

  • Ran:

    go test ./...
  • All tests pass successfully.

Fixes #417

Ansita20 added 2 commits May 9, 2026 03:13
Signed-off-by: ansita20 <ansitasingh20@gmail.com>
Signed-off-by: ansita20 <ansitasingh20@gmail.com>
Copilot AI review requested due to automatic review settings May 23, 2026 08:19
@github-actions
Copy link
Copy Markdown

👋 @Ansita20

Welcome to the Microcks community! 💖

Thanks and congrats 🎉 for opening your first pull request here! Be sure to follow the pull request template or please update it accordingly.

Hope you have a great time there!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR corrects typos in exported constructor names and improves path/test handling to make the CLI and watcher components more consistent and reliable.

Changes:

  • Rename NewCommadNewCommand and update the entrypoint call site.
  • Rename NewWatchMangerNewWatchManager and update call sites.
  • Normalize import file paths earlier; make TestDeleteContext write config into a per-test temp dir.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
watcher/main.go Updates watcher manager constructor call to the corrected name.
pkg/watcher/watchManager.go Renames exported constructor to NewWatchManager.
main.go Updates CLI command constructor call to NewCommand.
cmd/import.go Moves/expands path normalization before upload and watcher config updates.
cmd/context_test.go Uses t.TempDir() for isolated config file creation in tests.
cmd/cmd.go Renames exported CLI constructor to NewCommand.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/import.go
Comment on lines +134 to +137
// Normalize file path to match the watcher fsnotify events format.
if strings.HasPrefix(f, "./") {
f = strings.TrimPrefix(f, "./")
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the problem is that [string.HasPrefix(f,"./")] only matches POSIX ./ . it misses Windows .\ and doesn't normalize segments like foo/../bar or duplicate slashes so the fsnotify and other file APIs may produce paths with different separators or redundant segments. to fix this use [filepath.Clean] (OS-aware normalization) then trim a leading . + path separator.

Comment thread cmd/context_test.go Outdated
Comment on lines 41 to 44
testConfigFilePath := filepath.Join(t.TempDir(), "local.config")

//write the test config file
err := os.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm)
…ain.go

Signed-off-by: ansita20 <ansitasingh20@gmail.com>
@Ansita20 Ansita20 force-pushed the testdeletecontext-fails branch from 9bf3e25 to af23f9f Compare May 23, 2026 08:21
…e file mode

Signed-off-by: ansita20 <ansitasingh20@gmail.com>
@Syedowais312
Copy link
Copy Markdown
Contributor

The minor changes in context_test.go are already included in PR #385, so it would be better to revert/drop those changes from this PR to avoid duplication.

@Ansita20
Copy link
Copy Markdown
Author

The minor changes in context_test.go are already included in PR #385, so it would be better to revert/drop those changes from this PR to avoid duplication.

Oh ok thanks a lot I will be reverting these issues

Ansita20 added 2 commits May 23, 2026 16:51
Signed-off-by: ansita20 <ansitasingh20@gmail.com>
Signed-off-by: ansita20 <ansitasingh20@gmail.com>
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.

TestDeleteContext fails

3 participants