Repository: huggingface/transformers
Pull request: FIX: Faulty PEFT tests

Two [PEFT tests are actually failing](https://github.com/huggingface/transformers/actions/runs/14632303734/job/41057310419#step:13:241):

- `tests/peft_integration/test_peft_integration.py::PeftIntegrationTester::test_delete_adapter`
- `tests/peft_integration/test_peft_integration.py::PeftIntegrationTester::test_peft_pipeline_no_warning`

This must have been going on for some time but was apparently never noticed. The cause is that the tests themselves are faulty, the PEFT integration is correct in these cases.

## `test_delete_adapter`

The first faulty test was introduced by #34650. AFAICT, it should never have passed in the first place, the PEFT integration logic was not changed in the meantime. At this point, the logs for the PR CI are gone, so I'm not sure if the test passed back then or not.

## `test_peft_pipeline_no_warning`

This test was introduced in #36783 and IMO should also never have passed, as the [`self.assertNoLogs` context manager](https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertNoLogs) only returns `None`, thus the assert should never have worked (mea culpa for suggesting this code snippet). Here too, the CI logs are deleted by now, so I can't check if the test already failed back then.

## Note

Overall, I'm a bit concerned that those tests apparently never passed but the PRs were still merged. Also, it would be great if the PEFT maintainers could be alerted when the transformers PEFT tests fail. However, I could not find where the corresponding CI is defined.

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [x] Did you write any new necessary tests?

Changed files:
filename: tests/peft_integration/test_peft_integration.py
patch: @@ -13,6 +13,7 @@
 # limitations under the License.
 import importlib
 import os
+import re
 import tempfile
 import unittest
 
@@ -385,7 +386,7 @@ def test_delete_adapter(self):
 
                 # Delete remaining adapter
                 model.delete_adapter("adapter_2")
-                self.assertNotIn("adapter_2", model.peft_config)
+                self.assertFalse(hasattr(model, "peft_config"))
                 self.assertFalse(model._hf_peft_config_loaded)
 
                 # Re-add adapters for edge case tests
@@ -394,28 +395,28 @@ def test_delete_adapter(self):
 
                 # Attempt to delete multiple adapters at once
                 model.delete_adapter(["adapter_1", "adapter_2"])
-                self.assertNotIn("adapter_1", model.peft_config)
-                self.assertNotIn("adapter_2", model.peft_config)
+                self.assertFalse(hasattr(model, "peft_config"))
                 self.assertFalse(model._hf_peft_config_loaded)
 
                 # Test edge cases
+                msg = re.escape("No adapter loaded. Please load an adapter first.")
+                with self.assertRaisesRegex(ValueError, msg):
+                    model.delete_adapter("nonexistent_adapter")
+
+                model.add_adapter(peft_config_1, adapter_name="adapter_1")
+
                 with self.assertRaisesRegex(ValueError, "The following adapter\\(s\\) are not present"):
                     model.delete_adapter("nonexistent_adapter")
 
                 with self.assertRaisesRegex(ValueError, "The following adapter\\(s\\) are not present"):
                     model.delete_adapter(["adapter_1", "nonexistent_adapter"])
 
                 # Deleting with an empty list or None should not raise errors
-                model.add_adapter(peft_config_1, adapter_name="adapter_1")
                 model.add_adapter(peft_config_2, adapter_name="adapter_2")
                 model.delete_adapter([])  # No-op
                 self.assertIn("adapter_1", model.peft_config)
                 self.assertIn("adapter_2", model.peft_config)
 
-                model.delete_adapter(None)  # No-op
-                self.assertIn("adapter_1", model.peft_config)
-                self.assertIn("adapter_2", model.peft_config)
-
                 # Deleting duplicate adapter names in the list
                 model.delete_adapter(["adapter_1", "adapter_1"])
                 self.assertNotIn("adapter_1", model.peft_config)
@@ -832,7 +833,6 @@ def test_peft_pipeline_no_warning(self):
 
         # Input text for testing
         text = "Who is a Elon Musk?"
-        expected_error_msg = "The model 'PeftModel' is not supported for text-generation"
 
         model = AutoModelForCausalLM.from_pretrained(
             BASE_PATH,
@@ -849,7 +849,7 @@ def test_peft_pipeline_no_warning(self):
         # Create pipeline with PEFT model while capturing log output
         # Check that the warning message is not present in the logs
         pipeline_logger = logging.get_logger("transformers.pipelines.base")
-        with self.assertNoLogs(pipeline_logger, logging.ERROR) as cl:
+        with self.assertNoLogs(pipeline_logger, logging.ERROR):
             lora_generator = pipeline(
                 task="text-generation",
                 model=lora_model,
@@ -859,8 +859,3 @@ def test_peft_pipeline_no_warning(self):
 
             # Generate text to verify pipeline works
             _ = lora_generator(text)
-
-        # Check that the warning message is not present in the logs
-        self.assertNotIn(
-            expected_error_msg, cl.out, f"Error message '{expected_error_msg}' should not appear when using PeftModel"
-        )