Skip to content

Fix test warnings#39

Open
david-mears-2 wants to merge 5 commits intomainfrom
fix-orderly-warnings
Open

Fix test warnings#39
david-mears-2 wants to merge 5 commits intomainfrom
fix-orderly-warnings

Conversation

@david-mears-2
Copy link
Contributor

@david-mears-2 david-mears-2 commented Mar 5, 2026

There were lots of warnings in test output, which was polluting test outputs making them rather difficult to read.

Most of these were orderly warnings about a deprecated syntax.

You must assign calls to 'orderly_parameters()' to a variable
i The old behaviour of automatically copying parameters into the environment has been deprecated, and you should move to the new behaviour as soon as possible

Also: suppressed an expected warning - that line deliberately requests a non-existent job.

This was in response to the below warning, which was polluting test outputs making
them rather difficult to read:

You must assign calls to 'orderly_parameters()' to a variable
i The old behaviour of automatically copying parameters into the environment has been deprecated, and you should move to the new behaviour as soon as possible
@david-mears-2 david-mears-2 changed the title fix orderly warnings Fix test warnings Mar 5, 2026
Comment on lines 195 to +197
task_ids <- c(r1, "non-existent-id")

res <- q$get_status(task_ids, include_logs = FALSE)
res <- suppressWarnings(q$get_status(task_ids, include_logs = FALSE))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Warning is expected because the job does not exist

@david-mears-2 david-mears-2 requested a review from M-Kusumgar March 5, 2026 16:23
Copy link
Contributor

@M-Kusumgar M-Kusumgar left a comment

Choose a reason for hiding this comment

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

LGTM, could you add quiet = TRUE to install.packages when it installs "mime" in test-zzz-e2e.R or did you not do it given that its temporary and youll rip it out anyway? either way is fine!

This test was checking that null was equal to null, because
'res' dollar 'data' is null.

The initial intent was probably to check that the preexisting git repos
have not changed in this later response as compared to the earlier ones.
@david-mears-2
Copy link
Contributor Author

I've quieted that installation, and I've also added a commit that fixes a vacuous test.

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.

2 participants