Automated test plan #338
Draft
Automated test plan #338
+1,755
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edipascale
force-pushed
the
ema/peering-test
branch
2 times, most recently
from
dd013f7 to
6b55803
Compare
|
@edipascale could you please make a separate PR with the things refactoring in the testing.go so we can get it merged quickly and you'll not have to rebase |
edipascale
force-pushed
the
ema/peering-test
branch
5 times, most recently
from
beb5602 to
0cc35cd
Compare
Test Results16 tests 16 ✅ 33m 52s ⏱️ Results for commit d0571ca. ♻️ This comment has been updated with latest results. |
edipascale
force-pushed
the
ema/peering-test
branch
2 times, most recently
from
36a3864 to
08c5385
Compare
edipascale
force-pushed
the
ema/peering-test
branch
11 times, most recently
from
054c3d9 to
8283482
Compare
edipascale
force-pushed
the
ema/peering-test
branch
2 times, most recently
from
89eb0e7 to
fbc774c
Compare
edipascale
force-pushed
the
ema/peering-test
branch
from
fbc774c to
cfc1cbb
Compare
Frostman
requested changes
Mar 5, 2025
There was a problem hiding this comment.
Great job! I wasn't digging much into each test case, mainly looking for potential issues, unclear behavior, and general Go code issues.
Few notes for fixing in future PRs:
- I have a few questions about making sure that tests that are suitable for the VLAB will run
- failed cleanups in defer may leave fabric in a semi-broken state
- make SKIPed (b/c some requirements are missing) tests not FAIL (let's discuss)
- overall report in the could be a bit more clear (prev item probably contributes a lot to it)
| // 3. Set restricted flag in subnet-02 in vpc-02, test connectivity | ||
| // 4. Remove all restrictions | ||
| func (testCtx *VPCPeeringTestCtx) multiSubnetsIsolationTest(ctx context.Context) (bool, error) { | ||
| // FIXME: make sure to reset all changes after the test regardles of early failures |
There was a problem hiding this comment.
Should it be a TODO item for the future or you intended to fix it before the PR merged?
edipascale
force-pushed
the
ema/peering-test
branch
from
d0571ca to
d30c0b9
Compare
edipascale
force-pushed
the
ema/peering-test
branch
2 times, most recently
from
23f0b7f to
80668fb
Compare
Signed-off-by: Emanuele Di Pascale <[email protected]>
a PR with the label 'release-test' will trigger a run on the hlab Signed-off-by: Emanuele Di Pascale <[email protected]>
edipascale
force-pushed
the
ema/peering-test
branch
from
80668fb to
e2d0f80
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.