Skip to content

Add regression tests to CI#70

Merged
Araneidae merged 5 commits intomasterfrom
issue-14
Sep 11, 2025
Merged

Add regression tests to CI#70
Araneidae merged 5 commits intomasterfrom
issue-14

Conversation

@EmilioPeJu
Copy link
Copy Markdown
Contributor

@EmilioPeJu EmilioPeJu commented Sep 7, 2025 •
edited
Loading

  • Tests were adapted to some new changes (streaming tables and removal of .MIN).
  • A new github workflow called tests was added.
  • Fix the tests (by fixing some mistakes in the server).

In order for this to work, PR PandABlocks/PandABlocks-rootfs#70 needs to be merged and its associated docker image needs to be released.

This fixes #14.

@EmilioPeJu EmilioPeJu mentioned this pull request Sep 7, 2025
Closed
Araneidae
Araneidae requested changes Sep 8, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@Araneidae Araneidae left a comment

Choose a reason for hiding this comment

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

Something fishy about the nc option, I can't find it documented anywhere. I'm also doubtful about commit 1a54de5.

Comment thread tests/run_with_server Outdated
Comment thread server/attributes.c Outdated
@EmilioPeJu
Adjust regression tests to streaming table change (PR #63)
053cf8c 
- Appending to a table is no longer supported.
- Polled attributes need to be considered in *CHANGES commands.
- Consider new mode attribute.
@EmilioPeJu
Fix sim server hw_long_table_write (length should be number of words)
b4a89fb
@EmilioPeJu EmilioPeJu force-pushed the issue-14 branch 3 times, most recently from 26aba33 to fb3f11c Compare September 8, 2025 08:25
@EmilioPeJu
Copy link
Copy Markdown
Contributor Author

EmilioPeJu commented Sep 8, 2025 •
edited
Loading

The tests CI workflow is not passing because of missing netcat, this will work once the associated change in rootfs is merged (and the docker image released).

@Araneidae
Copy link
Copy Markdown
Contributor

Araneidae commented Sep 8, 2025

Where did the nc -N option come from? Looking through online man pages for nc I'm seeing a -N file for "Specifies file with pattern for UDP port scanning" in the Oracle page, which is clearly not what we want ... and I'm not seeing it anywhere else.

@EmilioPeJu
Copy link
Copy Markdown
Contributor Author

EmilioPeJu commented Sep 8, 2025

Where did the nc -N option come from? Looking through online man pages for nc I'm seeing a -N file for "Specifies file with pattern for UDP port scanning" in the Oracle page, which is clearly not what we want ... and I'm not seeing it anywhere else.

It's probably something they removed in newer versions, to find the option help, you could run the panda container and yum install netcat, then check the command help.

@EmilioPeJu
Copy link
Copy Markdown
Contributor Author

EmilioPeJu commented Sep 8, 2025

The docker image was updated and CI is now happy!

coretl
coretl reviewed Sep 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@coretl coretl left a comment

Choose a reason for hiding this comment

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

CI looks fine, I leave it to @Araneidae to decide if the server changes are reasonable

Araneidae
Araneidae requested changes Sep 11, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@Araneidae Araneidae left a comment

Choose a reason for hiding this comment

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

If the CI can be made to work for now without the change to attributes.c I'd like to handle this as a separate PR because I think there may be implications for the entire change mechanism.

@EmilioPeJu
Copy link
Copy Markdown
Contributor Author

EmilioPeJu commented Sep 11, 2025

The polled attributes fix has been split to PR #71, I dropped the commit from this one and added a new commit to workaround the test, this commit can be reverted once the other PR is merged

@Araneidae Araneidae merged commit 77a98bb into master Sep 11, 2025
8 checks passed
@Araneidae Araneidae deleted the issue-14 branch September 11, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coretl coretl coretl left review comments

@Araneidae Araneidae Araneidae approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add regression tests to CI

3 participants

@EmilioPeJu @Araneidae @coretl